From 41cca3f51f7c755b4e7bc09833ff30a401dbf579 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 14 Oct 2026 21:45:30 -0600 Subject: [PATCH 1/6] Add opt-in persistent vanilla voting NPCs --- .../votingplugin/VotingPluginMain.java | 46 ++ .../votingplugin/commands/CommandLoader.java | 9 + .../bencodez/votingplugin/config/Config.java | 3 + .../votingplugin/npc/VotingNPCCommands.java | 99 +++++ .../votingplugin/npc/VotingNPCDefinition.java | 29 ++ .../votingplugin/npc/VotingNPCManager.java | 403 ++++++++++++++++++ .../votingplugin/npc/VotingNPCRegistry.java | 199 +++++++++ .../votingplugin/npc/VotingNPCScheduler.java | 77 ++++ .../votingplugin/npc/VotingNPCStore.java | 200 +++++++++ VotingPlugin/src/main/resources/Config.yml | 6 + VotingPlugin/src/main/resources/plugin.yml | 4 + .../npc/VotingNPCCommandsTest.java | 132 ++++++ .../npc/VotingNPCManagerTest.java | 324 ++++++++++++++ .../npc/VotingNPCRegistryTest.java | 214 ++++++++++ .../npc/VotingNPCSchedulerTest.java | 78 ++++ .../votingplugin/npc/VotingNPCStoreTest.java | 141 ++++++ docs/voting-npcs.md | 96 +++++ 17 files changed, 2060 insertions(+) create mode 100644 VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java create mode 100644 VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCDefinition.java create mode 100644 VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java create mode 100644 VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java create mode 100644 VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCScheduler.java create mode 100644 VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java create mode 100644 VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java create mode 100644 VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java create mode 100644 VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java create mode 100644 VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCSchedulerTest.java create mode 100644 VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java create mode 100644 docs/voting-npcs.md diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/VotingPluginMain.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/VotingPluginMain.java index cde4d636b9..013d52b1ca 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/VotingPluginMain.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/VotingPluginMain.java @@ -156,6 +156,49 @@ public class VotingPluginMain extends AdvancedCorePlugin { + private final Object votingNPCLifecycle = new Object(); + private final java.util.concurrent.atomic.AtomicLong votingNPCConfiguration = new java.util.concurrent.atomic.AtomicLong(); + private volatile com.bencodez.votingplugin.npc.VotingNPCManager votingNPCManager; + + public com.bencodez.votingplugin.npc.VotingNPCManager getVotingNPCManager() { return votingNPCManager; } + + /** Optional NPC storage is lazy and asynchronous; configuration only schedules owner work. */ + private void refreshVotingNPCs() { + boolean requested = getConfigFile().isVotingNPCsEnabled(); + long generation = votingNPCConfiguration.incrementAndGet(); + if (!requested && votingNPCManager == null) return; + getBukkitScheduler().runTask(this, () -> { + com.bencodez.votingplugin.npc.VotingNPCManager manager; + synchronized (votingNPCLifecycle) { + if (!isEnabled() || generation != votingNPCConfiguration.get()) return; + if (votingNPCManager == null && requested) { + try { + var created = new com.bencodez.votingplugin.npc.VotingNPCManager(this); + Bukkit.getPluginManager().registerEvents(created, this); + votingNPCManager = created; + } catch (RuntimeException | LinkageError unsupported) { + getLogger().warning("Voting NPCs are unavailable on this server; normal voting is unaffected."); + return; + } + } + manager = votingNPCManager; + } + if (manager != null) manager.configure(requested); + }); + } + + private void closeVotingNPCs() { + com.bencodez.votingplugin.npc.VotingNPCManager manager; + synchronized (votingNPCLifecycle) { + votingNPCConfiguration.incrementAndGet(); + manager = votingNPCManager; + votingNPCManager = null; + } + if (manager != null) { + try { manager.close(); } catch (RuntimeException | LinkageError failure) { debug(failure); } + } + } + @Getter public static VotingPluginMain plugin; @@ -748,6 +791,7 @@ public void onPostLoad() { checkVotifier(); registerEvents(); refreshPlaceholderPlayerPresence(); + refreshVotingNPCs(); loadVoteBroadcast(); @@ -1961,6 +2005,7 @@ private void shutdownVoteReminders() { @Override public void onUnLoad() { + closeVotingNPCs(); placeholderPlayerPresence.clear(); stopBackendHostedControlLifecycle(); stopBackendControlConnectorLifecycle(); @@ -2172,6 +2217,7 @@ private void reloadPlugin(boolean userStorage, boolean reconcileHostedControl, loadDirectlyDefined(); + refreshVotingNPCs(); if (reconcileHostedControl) restartBackendControlConnector(); setUpdate(true); diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/commands/CommandLoader.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/commands/CommandLoader.java index bfbb23af86..9768296505 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/commands/CommandLoader.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/commands/CommandLoader.java @@ -475,6 +475,7 @@ public boolean isVotingPluginCommand(Player player, String cmd) { */ private void loadAdminVoteCommand() { plugin.setAdminVoteCommand(new ArrayList<>()); + com.bencodez.votingplugin.npc.VotingNPCCommands.register(plugin); plugin.getAdminVoteCommand().add(new CommandHandler(plugin, new String[] { "CurrentPluginTime" }, "VotingPlugin.Commands.AdminVote.CurrentPluginTime|" + adminPerm, "Current plugin time") { @@ -3299,6 +3300,14 @@ public void onInput(Player player, String value) { * Load tab complete. */ public void loadTabComplete() { + TabCompleteHandler.getInstance().addTabCompleteOption(new TabCompleteHandle("(npcid)", new ArrayList<>()) { + @Override public void reload() { updateReplacements(); } + @Override public void updateReplacements() { + var manager = plugin.getVotingNPCManager(); + setReplace(manager == null ? new ArrayList<>() : new ArrayList<>(manager.definitions().stream() + .map(com.bencodez.votingplugin.npc.VotingNPCDefinition::id).toList())); + } + }); ArrayList sites = new ArrayList<>(); for (VoteSite site : plugin.getVoteSiteManager().getVoteSitesEnabled()) { sites.add(site.getKey()); diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/config/Config.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/config/Config.java index af51b4995a..3f013372cb 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/config/Config.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/config/Config.java @@ -24,6 +24,9 @@ import lombok.Getter; public class Config extends YMLFile { + /** Opt-in persistent vanilla voting NPCs; missing configuration preserves legacy behavior. */ + public boolean isVotingNPCsEnabled() { return getData().getBoolean("VotingNPCs.Enabled", false); } + /** Explicit permission for executable deployment over private-network plaintext HTTP. */ public boolean getControlAllowInsecureHttpPluginDeployment() { diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java new file mode 100644 index 0000000000..e87d71e937 --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java @@ -0,0 +1,99 @@ +package com.bencodez.votingplugin.npc; + +import java.util.Arrays; +import java.util.UUID; +import java.util.concurrent.CompletionStage; + +import org.bukkit.Location; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; + +import com.bencodez.advancedcore.api.command.CommandHandler; +import com.bencodez.votingplugin.VotingPluginMain; +import com.bencodez.votingplugin.util.BukkitCompletionScheduler; + +/** Normal /av command handlers; no separate command map or player GUI registry. */ +public final class VotingNPCCommands { + private static final String PERMISSION = "VotingPlugin.Commands.AdminVote.NPC"; + private VotingNPCCommands() { } + + public static void register(VotingPluginMain plugin) { + add(plugin, new String[] { "NPC", "Create", "(string)" }, false); + add(plugin, new String[] { "NPC", "Create", "(string)", "(list)" }, false); + add(plugin, new String[] { "NPC", "List" }, true); + add(plugin, new String[] { "NPC", "Inspect", "(npcid)" }, true); + add(plugin, new String[] { "NPC", "Remove", "(npcid)" }, true); + } + + private static void add(VotingPluginMain plugin, String[] pattern, boolean console) { + plugin.getAdminVoteCommand().add(new CommandHandler(plugin, pattern, PERMISSION + "|VotingPlugin.Admin", + "Manage saved vanilla voting NPCs", console) { + @Override public void execute(CommandSender sender, String[] args) { + // CommandHandler invokes execute asynchronously. Capture location, + // permission and messages only after reaching the sender's owner. + String[] captured = args.clone(); + onSender(plugin, sender, () -> executeOwned(plugin, sender, captured)); + } + }); + } + + private static boolean allowed(CommandSender sender) { + return (!(sender instanceof Player player) || player.isOnline()) + && (sender.hasPermission(PERMISSION) || sender.hasPermission("VotingPlugin.Admin")); + } + + private static void onSender(VotingPluginMain plugin, CommandSender sender, Runnable action) { + Player player = sender instanceof Player candidate ? candidate : null; + BukkitCompletionScheduler.run(plugin, player, action, player == null ? action : () -> { }, () -> { }); + } + + private static void executeOwned(VotingPluginMain plugin, CommandSender sender, String[] args) { + if (!allowed(sender)) return; + VotingNPCManager manager = plugin.getVotingNPCManager(); + if (manager == null || !manager.available()) { + sender.sendMessage("Voting NPCs are disabled, unsupported, or waiting for valid storage. Check VotingNPCs.Enabled in Config.yml and reload."); + return; + } + switch (args[1].toLowerCase(java.util.Locale.ROOT)) { + case "create": + if (!(sender instanceof Player player)) { sender.sendMessage("Create a voting NPC from in game."); return; } + try { + Location at = player.getLocation(); + String name = args.length > 3 ? String.join(" ", Arrays.copyOfRange(args, 3, args.length)) : "&aVote"; + VotingNPCDefinition definition = new VotingNPCDefinition(args[2], UUID.randomUUID(), at.getWorld().getUID(), + at.getX(), at.getY(), at.getZ(), at.getYaw(), at.getPitch(), name); + confirm(plugin, sender, manager.create(definition), + ignored -> "Saved voting NPC '" + definition.id() + "'. Its villager appears when the area is loaded and available."); + } catch (IllegalArgumentException invalid) { + sender.sendMessage("Use a lowercase ID of 1-40 letters, digits, underscores or hyphens, and a name of 1-128 characters without controls."); + } + break; + case "remove": + confirm(plugin, sender, manager.remove(args[2]), removed -> removed + ? "Removed saved voting NPC '" + args[2] + "'; exact entity cleanup has been requested." + : "No voting NPC has that ID."); + break; + case "inspect": + manager.definitions().stream().filter(value -> value.id().equals(args[2])).findFirst().ifPresentOrElse(value -> + sender.sendMessage("Voting NPC " + value.id() + ": world " + value.world() + ", " + + value.x() + ", " + value.y() + ", " + value.z() + "; " + manager.runtimeState(value.id())), + () -> sender.sendMessage("No voting NPC has that ID.")); + break; + case "list": + sender.sendMessage("Saved voting NPCs: " + manager.definitions().size() + "/64; tracked runtime slots: " + manager.runtimeCount()); + for (var value : manager.definitions()) sender.sendMessage(value.id() + ": " + manager.runtimeState(value.id())); + break; + default: + sender.sendMessage("Use /av npc create [name], list, inspect , or remove ."); + } + } + + private static void confirm(VotingPluginMain plugin, CommandSender sender, CompletionStage completion, + java.util.function.Function message) { + completion.whenComplete((value, failure) -> onSender(plugin, sender, () -> { + if (!allowed(sender)) return; + sender.sendMessage(failure == null ? message.apply(value) + : "Voting NPC change was not confirmed. Check storage and reload before retrying; no success is assumed."); + })); + } +} diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCDefinition.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCDefinition.java new file mode 100644 index 0000000000..3a2ff19a3f --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCDefinition.java @@ -0,0 +1,29 @@ +package com.bencodez.votingplugin.npc; + +import java.util.Objects; +import java.util.UUID; +import java.util.regex.Pattern; + +/** Durable definition; it contains no live world, player, entity or reward state. */ +public record VotingNPCDefinition(String id, UUID identity, UUID world, double x, double y, double z, + float yaw, float pitch, String name) { + private static final Pattern ID = Pattern.compile("[a-z0-9][a-z0-9_-]{0,39}"); + + public VotingNPCDefinition { + if (id == null || !ID.matcher(id).matches()) throw new IllegalArgumentException("Invalid voting NPC ID"); + Objects.requireNonNull(identity, "identity"); + Objects.requireNonNull(world, "world"); + if (!Double.isFinite(x) || !Double.isFinite(y) || !Double.isFinite(z) + || Math.abs(x) > 30_000_000 || Math.abs(z) > 30_000_000 + || !Float.isFinite(yaw) || !Float.isFinite(pitch)) { + throw new IllegalArgumentException("Invalid voting NPC position"); + } + if (name == null || name.isEmpty() || name.length() > 128 + || name.codePoints().anyMatch(Character::isISOControl)) { + throw new IllegalArgumentException("Invalid voting NPC name"); + } + } + + public int chunkX() { return Math.floorDiv((int) Math.floor(x), 16); } + public int chunkZ() { return Math.floorDiv((int) Math.floor(z), 16); } +} diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java new file mode 100644 index 0000000000..16f5fea8d7 --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java @@ -0,0 +1,403 @@ +package com.bencodez.votingplugin.npc; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; + +import org.bukkit.Bukkit; +import org.bukkit.ChatColor; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Entity; +import org.bukkit.entity.LivingEntity; +import org.bukkit.entity.Player; +import org.bukkit.entity.Villager; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; +import org.bukkit.event.entity.EntityCombustEvent; +import org.bukkit.event.entity.EntityDamageEvent; +import org.bukkit.event.entity.EntityInteractEvent; +import org.bukkit.event.entity.EntityTargetEvent; +import org.bukkit.event.entity.EntityTeleportEvent; +import org.bukkit.event.entity.EntityTransformEvent; +import org.bukkit.event.player.PlayerInteractAtEntityEvent; +import org.bukkit.event.player.PlayerInteractEntityEvent; +import org.bukkit.event.player.PlayerQuitEvent; +import org.bukkit.event.vehicle.VehicleEnterEvent; +import org.bukkit.event.world.ChunkLoadEvent; +import org.bukkit.event.world.ChunkUnloadEvent; +import org.bukkit.event.world.WorldLoadEvent; +import org.bukkit.event.world.WorldUnloadEvent; +import org.bukkit.inventory.EquipmentSlot; + +import com.bencodez.votingplugin.VotingPluginMain; +import com.bencodez.votingplugin.util.BukkitCompletionScheduler; + +/** Persistent definitions, transient public villagers, and no vote/reward accounting. */ +public final class VotingNPCManager implements Listener, AutoCloseable { + private final VotingPluginMain plugin; + private final VotingNPCScheduler scheduler; + private final Consumer open; + private final Map resources = new ConcurrentHashMap<>(); + private final Map entities = new ConcurrentHashMap<>(); + private final Set unloadingChunks = ConcurrentHashMap.newKeySet(); + private final Set unloadingWorlds = ConcurrentHashMap.newKeySet(); + private final Map recentClicks = new LinkedHashMap<>(); + private volatile VotingNPCRegistry registry; + private volatile boolean enabled; + private volatile boolean closed; + private volatile long configuration; + private final AtomicBoolean warned = new AtomicBoolean(); + + private record ChunkKey(UUID world, int x, int z) { } + private static final class RuntimeNPC { + final VotingNPCDefinition definition; + final Location anchor; + final long configuration; + boolean closed, cleanupQueued, reconstructRequested; + Villager entity; + UUID entityId; + RuntimeNPC(VotingNPCDefinition definition, World world, long configuration) { + this.definition = definition; + this.configuration = configuration; + anchor = new Location(world, definition.x(), definition.y(), definition.z(), definition.yaw(), definition.pitch()); + } + } + + public VotingNPCManager(VotingPluginMain plugin) { + this(plugin, new VotingNPCScheduler(plugin), null, player -> openVotingSites(plugin, player)); + } + + static void openVotingSites(VotingPluginMain plugin, Player player) { + // Reuse the normal command's permissions, asynchronous user loading, + // configured GUI, cooldown display, and URL handling; no root alias. + for (var handler : plugin.getVoteCommand()) { + if (handler.runCommand(player, new String[] { "URL" })) return; + } + player.sendMessage("Voting-site selection is unavailable."); + } + + VotingNPCManager(VotingPluginMain plugin, VotingNPCScheduler scheduler, VotingNPCRegistry registry, Consumer open) { + this.plugin = plugin; + this.scheduler = scheduler; + this.registry = registry; + this.open = open; + } + + public static boolean supported() { + try { + Entity.class.getMethod("setPersistent", boolean.class); + Entity.class.getMethod("setGravity", boolean.class); + Entity.class.getMethod("setInvulnerable", boolean.class); + LivingEntity.class.getMethod("setAI", boolean.class); + LivingEntity.class.getMethod("setCollidable", boolean.class); + World.class.getMethod("spawn", Location.class, Class.class, Consumer.class); + return true; + } catch (ReflectiveOperationException | LinkageError unavailable) { return false; } + } + + /** Configuration callers do no file I/O and do not wait for storage. */ + public void configure(boolean requested) { + VotingNPCRegistry owner; + CompletionStage> loading; + long generation; + boolean activate; + synchronized (this) { + if (closed) return; + generation = ++configuration; + activate = requested && supported(); + enabled = activate; + if (!activate) { owner = null; loading = null; } + else { + if (registry == null) { + registry = new VotingNPCRegistry(plugin.getDataFolder().toPath().resolve("VotingNPCs.json")); + loading = registry.initialLoad(); + } else loading = registry.reload(); + owner = registry; + } + } + if (!activate) { + for (RuntimeNPC npc : resources.values()) retire(npc); + if (requested) report(new IllegalStateException("Vanilla voting NPC API is unavailable")); + return; + } + loading.whenComplete((definitions, failure) -> { + synchronized (VotingNPCManager.this) { + if (closed || !enabled || generation != configuration || registry != owner) return; + } + if (failure != null) { + for (RuntimeNPC npc : resources.values()) retire(npc); + report(failure); + } else { + warned.set(false); + reconcile(); + } + }); + } + + public boolean available() { + VotingNPCRegistry owner = registry; + return !closed && enabled && owner != null && owner.available(); + } + + public List definitions() { + VotingNPCRegistry owner = registry; + return owner == null ? List.of() : owner.snapshot(); + } + + /** Returns durable definition completion; runtime creation can still await a loaded chunk. */ + public CompletionStage create(VotingNPCDefinition definition) { + VotingNPCRegistry owner = registry; + if (!available()) return CompletableFuture.failedFuture(new IllegalStateException("Enable VotingNPCs and load valid storage first")); + CompletionStage completion = owner.create(definition); + completion.whenComplete((value, failure) -> mutationFinished(owner, failure)); + return completion; + } + + public CompletionStage remove(String id) { + VotingNPCRegistry owner = registry; + if (!available()) return CompletableFuture.failedFuture(new IllegalStateException("Enable VotingNPCs and load valid storage first")); + CompletionStage completion = owner.remove(id); + completion.whenComplete((value, failure) -> mutationFinished(owner, failure)); + return completion; + } + + private void mutationFinished(VotingNPCRegistry owner, Throwable failure) { + if (owner != registry || closed) return; + if (failure == null) reconcile(); + else if (!owner.available()) { + for (RuntimeNPC npc : resources.values()) retire(npc); + report(failure); + } + } + + private boolean current(RuntimeNPC npc) { + return available() && definitions().contains(npc.definition); + } + + private void reconcile() { + List snapshot = definitions(); + Set chunks = new HashSet<>(); + Set worlds = new HashSet<>(); + for (var value : snapshot) { chunks.add(key(value)); worlds.add(value.world()); } + unloadingChunks.retainAll(chunks); + unloadingWorlds.retainAll(worlds); + for (RuntimeNPC npc : resources.values()) if (!current(npc)) retire(npc); + if (!available()) return; + scheduler.global(() -> { + if (!available()) return; + for (var value : definitions()) ensure(value); + }, this::report); + } + + private static ChunkKey key(VotingNPCDefinition value) { return new ChunkKey(value.world(), value.chunkX(), value.chunkZ()); } + + private void ensure(VotingNPCDefinition value) { + if (!available() || !definitions().contains(value) || unloadingWorlds.contains(value.world()) || unloadingChunks.contains(key(value))) return; + World world = Bukkit.getWorld(value.world()); + if (world == null) return; + RuntimeNPC npc = new RuntimeNPC(value, world, configuration); + RuntimeNPC previous; + synchronized (resources) { + previous = resources.get(value.id()); + if (previous == null) { + if (resources.size() >= VotingNPCStore.MAX_NPCS) return; + resources.put(value.id(), npc); + } + } + if (previous != null) { + synchronized (previous) { if (previous.closed) previous.reconstructRequested = true; } + return; + } + scheduler.region(npc.anchor, () -> spawn(npc), failure -> { retire(npc); report(failure); }); + } + + private boolean loadedArea(RuntimeNPC npc) { + World world = npc.anchor.getWorld(); + if (Bukkit.getWorld(npc.definition.world()) != world || unloadingWorlds.contains(npc.definition.world()) + || unloadingChunks.contains(key(npc.definition))) return false; + for (double dx : new double[] { -.5, .5 }) for (double dz : new double[] { -.5, .5 }) { + Location edge = npc.anchor.clone().add(dx, 0, dz); + if (!scheduler.owns(edge) || !world.isChunkLoaded(edge.getBlockX() >> 4, edge.getBlockZ() >> 4)) return false; + } + return true; + } + + private void spawn(RuntimeNPC npc) { + try { + synchronized (npc) { + if (npc.closed || !current(npc) || !loadedArea(npc)) { forget(npc); return; } + if (npc.entity != null) return; + Villager spawned = npc.anchor.getWorld().spawn(npc.anchor, Villager.class, villager -> { + npc.entity = villager; + npc.entityId = villager.getUniqueId(); + entities.put(npc.entityId, npc); + villager.setPersistent(false); + villager.setAI(false); + villager.setGravity(false); + villager.setInvulnerable(true); + villager.setSilent(true); + villager.setCollidable(false); + villager.setCanPickupItems(false); + villager.setRemoveWhenFarAway(false); + villager.setCustomName(ChatColor.translateAlternateColorCodes('&', npc.definition.name())); + villager.setCustomNameVisible(true); + }); + if (spawned != npc.entity || !spawned.isValid() || !current(npc) || npc.closed) { + throw new IllegalStateException("Voting NPC spawn was cancelled or superseded"); + } + } + } catch (RuntimeException | LinkageError failure) { retire(npc); report(failure); } + } + + private void retire(RuntimeNPC npc) { + Villager entity; + synchronized (npc) { + npc.closed = true; + entity = npc.entity; + if (entity == null) { forget(npc); return; } + if (npc.cleanupQueued) return; + npc.cleanupQueued = true; + } + scheduler.entity(entity, () -> { + try { entity.remove(); forget(npc); } + catch (RuntimeException | LinkageError failure) { cleanupFailed(npc, failure); } + }, () -> forget(npc), failure -> cleanupFailed(npc, failure)); + } + + private void cleanupFailed(RuntimeNPC npc, Throwable failure) { + synchronized (npc) { npc.cleanupQueued = false; } + // Keep the exact identity and capacity reservation. Rejected scheduling + // is not successful removal and must not permit duplicate reconstruction. + report(failure); + } + + private void forget(RuntimeNPC npc) { + boolean reconstruct; + synchronized (npc) { + reconstruct = npc.entity != null && (npc.reconstructRequested || npc.configuration != configuration + || definitions().stream().anyMatch(value -> value.id().equals(npc.definition.id()) && !value.equals(npc.definition))); + if (npc.entityId != null) entities.remove(npc.entityId, npc); + npc.entity = null; + resources.remove(npc.definition.id(), npc); + } + // A successor sharing this configured ID must wait for exact predecessor + // cleanup. Missing/unloaded spawn attempts do not recursively retry. + if (reconstruct && available()) scheduler.global(() -> { + for (var value : definitions()) if (value.id().equals(npc.definition.id())) ensure(value); + }, this::report); + } + + private void report(Throwable failure) { + if (warned.compareAndSet(false, true)) { + try { + plugin.getLogger().warning("Voting NPCs could not complete an operation; definitions are retained. Check API support and VotingNPCs.json, then reload. Cause: " + + failure.getClass().getSimpleName()); + } catch (RuntimeException ignored) { /* Reporting must not prevent independent cleanup. */ } + } + } + + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void interact(PlayerInteractEntityEvent event) { + RuntimeNPC npc = entities.get(event.getRightClicked().getUniqueId()); + if (npc == null) return; + boolean cancelled = event.isCancelled(); + event.setCancelled(true); // Only this exact owned entity; never trading. + if (cancelled || event.getHand() != EquipmentSlot.HAND || !current(npc)) return; + synchronized (npc) { if (npc.closed) { retire(npc); return; } } + Player player = event.getPlayer(); + long now = System.nanoTime(); + synchronized (recentClicks) { + recentClicks.entrySet().removeIf(entry -> now - entry.getValue() >= 300_000_000L); + if (recentClicks.containsKey(player.getUniqueId()) || recentClicks.size() >= 256) return; + recentClicks.put(player.getUniqueId(), now); + } + BukkitCompletionScheduler.run(plugin, player, () -> { + if (!player.isOnline() || !current(npc) || !player.getWorld().getUID().equals(npc.definition.world()) + || player.getLocation().distanceSquared(npc.anchor) > 36) return; + synchronized (npc) { if (npc.closed) return; } + open.accept(player); + }, () -> { }, () -> { }); + } + + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void interactAt(PlayerInteractAtEntityEvent event) { interact(event); } + private boolean owned(Entity entity) { return entities.containsKey(entity.getUniqueId()); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void damage(EntityDamageEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void combust(EntityCombustEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void teleport(EntityTeleportEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void transform(EntityTransformEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void target(EntityTargetEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void blockInteract(EntityInteractEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) + public void vehicle(VehicleEnterEvent event) { if (owned(event.getEntered())) event.setCancelled(true); } + @EventHandler public void quit(PlayerQuitEvent event) { synchronized (recentClicks) { recentClicks.remove(event.getPlayer().getUniqueId()); } } + + @EventHandler(priority = EventPriority.MONITOR) + public void chunkLoad(ChunkLoadEvent event) { + UUID world = event.getWorld().getUID(); + unloadingChunks.remove(new ChunkKey(world, event.getChunk().getX(), event.getChunk().getZ())); + for (var value : definitions()) if (value.world().equals(world) + && Math.abs(value.chunkX() - event.getChunk().getX()) <= 1 + && Math.abs(value.chunkZ() - event.getChunk().getZ()) <= 1) ensure(value); + } + + @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) + public void chunkUnload(ChunkUnloadEvent event) { + ChunkKey chunk = new ChunkKey(event.getWorld().getUID(), event.getChunk().getX(), event.getChunk().getZ()); + if (definitions().stream().noneMatch(value -> key(value).equals(chunk))) return; + unloadingChunks.add(chunk); + for (RuntimeNPC npc : resources.values()) if (key(npc.definition).equals(chunk)) retire(npc); + } + + @EventHandler(priority = EventPriority.MONITOR) + public void worldLoad(WorldLoadEvent event) { + UUID world = event.getWorld().getUID(); + if (definitions().stream().noneMatch(value -> value.world().equals(world))) return; + unloadingWorlds.remove(world); + reconcile(); + } + + @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) + public void worldUnload(WorldUnloadEvent event) { + UUID world = event.getWorld().getUID(); + if (definitions().stream().noneMatch(value -> value.world().equals(world))) return; + unloadingWorlds.add(world); + for (RuntimeNPC npc : resources.values()) if (npc.definition.world().equals(world)) retire(npc); + } + + public int runtimeCount() { return resources.size(); } + + public String runtimeState(String id) { + RuntimeNPC npc = resources.get(id); + if (npc == null) return "waiting for a loaded, available area"; + synchronized (npc) { + if (npc.closed) return "cleanup pending"; + return npc.entity == null ? "creation scheduled" : "villager created"; + } + } + + @Override public void close() { + synchronized (this) { if (closed) return; closed = true; enabled = false; ++configuration; } + VotingNPCRegistry owner = registry; + if (owner != null) owner.close(); + for (RuntimeNPC npc : new ArrayList<>(resources.values())) retire(npc); + synchronized (recentClicks) { recentClicks.clear(); } + } +} diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java new file mode 100644 index 0000000000..e4297f59b7 --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java @@ -0,0 +1,199 @@ +package com.bencodez.votingplugin.npc; + +import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.OverlappingFileLockException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; + +/** + * One bounded, daemon file owner. Completion means the definition was persisted, + * not that a world entity has spawned. Callbacks have no player/region ownership. + */ +final class VotingNPCRegistry implements AutoCloseable { + private final Object admission = new Object(); + private final ArrayBlockingQueue queue = new ArrayBlockingQueue<>(128); + private final CountDownLatch stopped = new CountDownLatch(1); + private final VotingNPCStore store; + private final Path lockPath; + private final CompletableFuture> initial; + private volatile Map definitions = Map.of(); + private volatile boolean available; + private volatile boolean closed; + // Only the file worker accesses these resources. + private FileChannel lockChannel; + private FileLock lease; + private boolean storageReady; + + VotingNPCRegistry(Path file) { + this(file, new VotingNPCStore(file)); + } + + VotingNPCRegistry(Path file, VotingNPCStore store) { + this.store = store; + lockPath = file.toAbsolutePath().normalize().resolveSibling(file.getFileName() + ".lock"); + initial = submit(false, this::readDefinitions); + Thread thread = new Thread(this::work, "VotingPlugin-NPC-storage"); + thread.setDaemon(true); + try { thread.start(); } + catch (RuntimeException | Error failure) { + closed = true; + initial.completeExceptionally(failure); + stopped.countDown(); + throw failure; + } + } + + CompletionStage> initialLoad() { return initial; } + boolean available() { return !closed && available; } + List snapshot() { return List.copyOf(definitions.values()); } + + CompletionStage> reload() { + synchronized (admission) { + if (closed) return rejected("Voting NPC registry is closed"); + var completion = submit(false, this::readDefinitions); + // A rejected reload changes nothing. The worker publishes readiness + // under this same short lock, after admission has finished. + if (!completion.isCompletedExceptionally()) available = false; + return completion; + } + } + + CompletionStage create(VotingNPCDefinition definition) { + return submit(true, () -> { + if (definition == null) throw new IllegalArgumentException("Voting NPC definition is required"); + if (definitions.containsKey(definition.id())) throw new IllegalArgumentException("Voting NPC ID already exists"); + if (definitions.values().stream().anyMatch(value -> value.identity().equals(definition.identity()))) { + throw new IllegalArgumentException("Voting NPC identity already exists"); + } + if (definitions.size() >= VotingNPCStore.MAX_NPCS) throw new RejectedExecutionException("Voting NPC definition limit reached"); + var next = new LinkedHashMap<>(definitions); + next.put(definition.id(), definition); + publish(next); + return definition; + }); + } + + CompletionStage remove(String id) { + return submit(true, () -> { + if (!definitions.containsKey(id)) return false; + var next = new LinkedHashMap<>(definitions); + next.remove(id); + publish(next); + return true; + }); + } + + private List readDefinitions() throws IOException { + try { + ownFile(); + List loaded = store.load(); + var next = new LinkedHashMap(); + for (VotingNPCDefinition value : loaded) next.put(value.id(), value); + definitions = java.util.Collections.unmodifiableMap(next); + storageReady = true; + synchronized (admission) { available = true; } + return loaded; + } catch (IOException | RuntimeException failure) { + storageReady = false; + synchronized (admission) { available = false; } + throw failure; + } + } + + private void publish(LinkedHashMap next) throws IOException { + // The worker holds the process-independent lease through shutdown drain. + try { store.save(List.copyOf(next.values())); } + catch (IOException | RuntimeException | Error failure) { + // A failed write may already be visible. Neither later admitted + // mutations nor a new request may overwrite it from the old map. + storageReady = false; + synchronized (admission) { available = false; } + throw failure; + } + definitions = java.util.Collections.unmodifiableMap(next); + } + + private CompletableFuture submit(boolean needsReady, Callable action) { + synchronized (admission) { + if (closed || (needsReady && !available)) return rejected("Voting NPC storage is not available"); + var completion = new CompletableFuture(); + Runnable task = () -> { + try { + if (needsReady && !storageReady) throw new RejectedExecutionException("Reload valid voting NPC storage before editing"); + completion.complete(action.call()); + } + catch (Throwable failure) { completion.completeExceptionally(failure); } + }; + if (!queue.offer(task)) completion.completeExceptionally(new RejectedExecutionException("Voting NPC storage queue is full")); + return completion; + } + } + + private static CompletableFuture rejected(String message) { + return CompletableFuture.failedFuture(new RejectedExecutionException(message)); + } + + private void ownFile() throws IOException { + if (lease != null && lease.isValid()) return; + for (Path parent = lockPath.getParent(); parent != null; parent = parent.getParent()) { + if (Files.isSymbolicLink(parent)) throw new IOException("Voting NPC owner path cannot cross a symbolic link"); + } + FileChannel candidate = FileChannel.open(lockPath, StandardOpenOption.CREATE, + StandardOpenOption.WRITE, LinkOption.NOFOLLOW_LINKS); + try { + FileLock acquired; + try { acquired = candidate.tryLock(); } + catch (OverlappingFileLockException overlap) { throw new IOException("Voting NPC file still has an active owner", overlap); } + if (acquired == null) throw new IOException("Voting NPC file still has an active owner"); + lockChannel = candidate; + lease = acquired; + } finally { + if (lockChannel != candidate) candidate.close(); + } + } + + private void work() { + try { + while (!closed || !queue.isEmpty()) { + Runnable task; + try { task = queue.take(); } + catch (InterruptedException interrupted) { continue; } + task.run(); + } + } finally { + available = false; + try { + if (lockChannel != null) lockChannel.close(); + } catch (IOException ignored) { + // Closing this channel releases its OS lease; never do this I/O + // under an admission or executor lock on a gameplay caller. + } finally { stopped.countDown(); } + } + } + + /** Stops admission without blocking or interrupting an accepted file write. */ + @Override public void close() { + synchronized (admission) { + if (closed) return; + closed = true; + queue.offer(() -> { }); // Wake an idle worker; a full queue already wakes it. + } + } + + /** Test/owner-worker observation only; gameplay callers must never wait here. */ + boolean awaitClosed(long time, TimeUnit unit) throws InterruptedException { return stopped.await(time, unit); } +} diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCScheduler.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCScheduler.java new file mode 100644 index 0000000000..a57cd61ed1 --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCScheduler.java @@ -0,0 +1,77 @@ +package com.bencodez.votingplugin.npc; + +import java.lang.reflect.Method; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; + +import org.bukkit.Bukkit; +import org.bukkit.Location; +import org.bukkit.entity.Entity; + +import com.bencodez.votingplugin.VotingPluginMain; +import com.bencodez.votingplugin.util.BukkitCompletionScheduler; + +/** World creation uses a region; subsequent cleanup follows the actual entity. */ +class VotingNPCScheduler { + private final VotingPluginMain plugin; + private final Method locationOwner; + private final Method entityOwner; + + VotingNPCScheduler(VotingPluginMain plugin) { + this.plugin = plugin; + Method location = null, entity = null; + try { + Class.forName("io.papermc.paper.threadedregions.RegionizedServer"); + location = Bukkit.class.getMethod("isOwnedByCurrentRegion", Location.class); + entity = Bukkit.class.getMethod("isOwnedByCurrentRegion", Entity.class); + } catch (ClassNotFoundException ordinaryServer) { + // Spigot/Paper have a single world-owner thread. + } catch (ReflectiveOperationException unavailable) { + throw new IllegalStateException("Voting NPC region ownership API is unavailable", unavailable); + } + locationOwner = location; + entityOwner = entity; + } + + boolean owns(Location location) { return owns(locationOwner, location); } + boolean owns(Entity entity) { return owns(entityOwner, entity); } + + private boolean owns(Method method, Object value) { + if (method == null) return Bukkit.isPrimaryThread(); + try { return Boolean.TRUE.equals(method.invoke(null, value)); } + catch (ReflectiveOperationException failure) { throw new IllegalStateException("Cannot verify voting NPC ownership", failure); } + } + + void global(Runnable action, Consumer failure) { + try { + plugin.getBukkitScheduler().getFoliaLib().getImpl().runNextTick(ignored -> action.run()) + .whenComplete((ignored, failed) -> { if (failed != null) failure.accept(failed); }); + } catch (RuntimeException | LinkageError rejected) { failure.accept(rejected); } + } + + void region(Location anchor, Runnable action, Consumer failure) { + try { + if (owns(anchor)) action.run(); + else plugin.getBukkitScheduler().getFoliaLib().getImpl().runAtLocation(anchor, ignored -> action.run()) + .whenComplete((ignored, failed) -> { if (failed != null) failure.accept(failed); }); + } catch (RuntimeException | LinkageError rejected) { failure.accept(rejected); } + } + + void entity(Entity entity, Runnable action, Runnable retired, Consumer failure) { + AtomicBoolean executed = new AtomicBoolean(); + Runnable run = () -> { if (executed.compareAndSet(false, true)) action.run(); }; + Runnable gone = () -> { if (executed.compareAndSet(false, true)) retired.run(); }; + try { + if (owns(entity)) { run.run(); return; } + plugin.getBukkitScheduler().getFoliaLib().getImpl().runAtEntityWithFallback(entity, + ignored -> run.run(), gone).whenComplete((status, failed) -> { + if (failed != null) failure.accept(failed); + else if (status instanceof Enum result && "ENTITY_RETIRED".equals(result.name())) gone.run(); + else if (!BukkitCompletionScheduler.isSuccessfulEntityTaskResult(status)) { + // Scheduler retirement does not prove the entity was removed. + failure.accept(new IllegalStateException("Voting NPC entity scheduler is unavailable")); + } + }); + } catch (RuntimeException | LinkageError rejected) { failure.accept(rejected); } + } +} diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java new file mode 100644 index 0000000000..84c8658ee6 --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java @@ -0,0 +1,200 @@ +package com.bencodez.votingplugin.npc; + +import java.io.IOException; +import java.io.StringReader; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CodingErrorAction; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.UUID; + +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonToken; +import com.google.gson.stream.JsonWriter; +import com.bencodez.votingplugin.util.DurableFiles; + +/** Single storage-worker owner. Never call this file boundary on a gameplay thread. */ +class VotingNPCStore { + static final int MAX_NPCS = 64; + private static final int MAX_BYTES = 128 * 1024; + private static final Set FIELDS = Set.of("id", "identity", "world", "x", "y", "z", "yaw", "pitch", "name"); + private final Path file; + private boolean initialized; + private boolean uncertain; + + VotingNPCStore(Path file) { this.file = file.toAbsolutePath().normalize(); } + + synchronized List load() throws IOException { + initialized = false; + checkParent(); + BasicFileAttributes attributes; + try { attributes = Files.readAttributes(file, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); } + catch (NoSuchFileException missing) { + if (uncertain) throw new IOException("Uncertain voting NPC definitions are missing", missing); + initialized = true; + return List.of(); + } + if (!attributes.isRegularFile() || attributes.isSymbolicLink() || attributes.size() > MAX_BYTES) { + throw new IOException("Voting NPC definitions are not a bounded regular file"); + } + byte[] bytes; + try (var input = Files.newInputStream(file, LinkOption.NOFOLLOW_LINKS)) { + bytes = input.readNBytes(MAX_BYTES + 1); + } + if (bytes.length > MAX_BYTES) throw new IOException("Voting NPC definitions exceed limit"); + String text; + try { text = StandardCharsets.UTF_8.newDecoder().onMalformedInput(CodingErrorAction.REPORT) + .onUnmappableCharacter(CodingErrorAction.REPORT).decode(ByteBuffer.wrap(bytes)).toString(); } + catch (CharacterCodingException invalid) { throw new IOException("Invalid voting NPC encoding", invalid); } + List result = decode(text); + uncertain = false; + initialized = true; + return result; + } + + synchronized void save(List definitions) throws IOException { + if (!initialized || uncertain) throw new IOException("Load valid voting NPC definitions before editing"); + validate(definitions); + byte[] bytes = encode(definitions).getBytes(StandardCharsets.UTF_8); + if (bytes.length > MAX_BYTES) throw new IOException("Voting NPC definitions exceed limit"); + checkParent(); + if (Files.isSymbolicLink(file)) throw new IOException("Voting NPC definitions cannot be a symbolic link"); + Path staged = Files.createTempFile(file.getParent(), ".voting-npcs-", ".tmp"); + boolean published = false; + Throwable primary = null; + try { + try (FileChannel output = FileChannel.open(staged, StandardOpenOption.WRITE)) { + ByteBuffer buffer = ByteBuffer.wrap(bytes); + while (buffer.hasRemaining()) output.write(buffer); + output.force(true); + } + Files.move(staged, file, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + published = true; + forceDirectory(file.getParent()); + } catch (IOException | RuntimeException | Error failure) { + // A visible replacement followed by a failed directory sync is not a + // rollback. Fence later mutations rather than overwrite it from old state. + if (published) uncertain = true; + primary = failure; + throw failure; + } finally { + try { Files.deleteIfExists(staged); } + catch (IOException cleanupFailure) { + if (primary != null) primary.addSuppressed(cleanupFailure); + else { + if (published) uncertain = true; + throw cleanupFailure; + } + } + } + } + + private void checkParent() throws IOException { + for (Path parent = file.getParent(); parent != null; parent = parent.getParent()) { + if (Files.isSymbolicLink(parent)) throw new IOException("Voting NPC storage cannot cross a symbolic link"); + } + if (!Files.isDirectory(file.getParent(), LinkOption.NOFOLLOW_LINKS)) { + throw new IOException("Voting NPC data directory is unavailable"); + } + } + + /** Kept as a narrow I/O seam so failure-after-publication can be tested. */ + void forceDirectory(Path directory) throws IOException { + DurableFiles.forceDirectory(directory); + } + + private static void validate(List values) throws IOException { + if (values == null || values.size() > MAX_NPCS) throw new IOException("Voting NPC definition limit exceeded"); + Set ids = new HashSet<>(); + Set identities = new HashSet<>(); + for (VotingNPCDefinition value : values) { + if (value == null || !ids.add(value.id()) || !identities.add(value.identity())) { + throw new IOException("Duplicate or missing voting NPC identity"); + } + } + } + + private static String encode(List values) throws IOException { + var text = new java.io.StringWriter(); + try (JsonWriter writer = new JsonWriter(text)) { + writer.setIndent(" "); + writer.beginObject().name("version").value(1).name("npcs").beginArray(); + for (VotingNPCDefinition value : values) { + writer.beginObject().name("id").value(value.id()).name("identity").value(value.identity().toString()) + .name("world").value(value.world().toString()).name("x").value(value.x()).name("y").value(value.y()) + .name("z").value(value.z()).name("yaw").value(value.yaw()).name("pitch").value(value.pitch()) + .name("name").value(value.name()).endObject(); + } + writer.endArray().endObject(); + } + return text.toString() + "\n"; + } + + private static List decode(String text) throws IOException { + try (JsonReader reader = new JsonReader(new StringReader(text))) { + Set keys = new HashSet<>(); + List values = null; + reader.beginObject(); + while (reader.hasNext()) { + String key = reader.nextName(); + if (!keys.add(key)) throw new IOException("Duplicate voting NPC document field"); + if (key.equals("version")) { + if (reader.peek() != JsonToken.NUMBER || !reader.nextString().equals("1")) { + throw new IOException("Unsupported voting NPC document version"); + } + } else if (key.equals("npcs")) { + values = new ArrayList<>(); + reader.beginArray(); + while (reader.hasNext()) { + if (values.size() >= MAX_NPCS) throw new IOException("Voting NPC definition limit exceeded"); + values.add(readDefinition(reader)); + } + reader.endArray(); + } else throw new IOException("Unknown voting NPC document field"); + } + reader.endObject(); + if (!keys.equals(Set.of("version", "npcs")) || reader.peek() != JsonToken.END_DOCUMENT) { + throw new IOException("Incomplete or trailing voting NPC document"); + } + validate(values); + return List.copyOf(values); + } catch (IllegalArgumentException | IllegalStateException invalid) { + throw new IOException("Invalid voting NPC definitions", invalid); + } + } + + private static VotingNPCDefinition readDefinition(JsonReader reader) throws IOException { + var fields = new java.util.HashMap(); + reader.beginObject(); + while (reader.hasNext()) { + String field = reader.nextName(); + if (!FIELDS.contains(field) || fields.containsKey(field)) throw new IOException("Unknown or duplicate voting NPC field"); + boolean numeric = Set.of("x", "y", "z", "yaw", "pitch").contains(field); + if (reader.peek() != (numeric ? JsonToken.NUMBER : JsonToken.STRING)) throw new IOException("Invalid voting NPC field type"); + fields.put(field, reader.nextString()); + } + reader.endObject(); + if (!fields.keySet().equals(FIELDS)) throw new IOException("Missing voting NPC field"); + return new VotingNPCDefinition(fields.get("id"), uuid(fields.get("identity")), uuid(fields.get("world")), + Double.parseDouble(fields.get("x")), Double.parseDouble(fields.get("y")), Double.parseDouble(fields.get("z")), + Float.parseFloat(fields.get("yaw")), Float.parseFloat(fields.get("pitch")), fields.get("name")); + } + + private static UUID uuid(String text) { + UUID value = UUID.fromString(text); + if (!value.toString().equals(text)) throw new IllegalArgumentException("Noncanonical voting NPC identity"); + return value; + } +} diff --git a/VotingPlugin/src/main/resources/Config.yml b/VotingPlugin/src/main/resources/Config.yml index 1a44764824..3dd646c3f0 100644 --- a/VotingPlugin/src/main/resources/Config.yml +++ b/VotingPlugin/src/main/resources/Config.yml @@ -1256,3 +1256,9 @@ DisableUpdateChecking: false # Disable checking permissions on tab complete # Could increase performance slightly, but probably not DisableAdvancedTab: false + +# Optional public vanilla villagers opening the existing voting-site menu. +# Enable and /av reload, then /av npc create [name]. No Citizens required. +# Definitions live in VotingNPCs.json; disabling keeps them but removes live NPCs. +VotingNPCs: + Enabled: false diff --git a/VotingPlugin/src/main/resources/plugin.yml b/VotingPlugin/src/main/resources/plugin.yml index 633dc78417..94f5dcc414 100644 --- a/VotingPlugin/src/main/resources/plugin.yml +++ b/VotingPlugin/src/main/resources/plugin.yml @@ -195,3 +195,7 @@ permissions: VotingPlugin.Login.RemindVotes.All: description: Remind if player can vote on all votesites, rather than any default: false + + VotingPlugin.Commands.AdminVote.NPC: + description: Create, inspect, list and remove saved vanilla voting NPCs + default: op diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java new file mode 100644 index 0000000000..9fffa3f9a0 --- /dev/null +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java @@ -0,0 +1,132 @@ +package com.bencodez.votingplugin.npc; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.LinkedBlockingQueue; + +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Entity; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import com.bencodez.advancedcore.api.command.CommandHandler; +import com.bencodez.simpleapi.scheduler.BukkitScheduler; +import com.bencodez.votingplugin.VotingPluginMain; + +class VotingNPCCommandsTest { + private static class Fixture { + final VotingPluginMain plugin = mock(VotingPluginMain.class, RETURNS_DEEP_STUBS); + final BukkitScheduler scheduler = mock(BukkitScheduler.class); + final VotingNPCManager manager = mock(VotingNPCManager.class); + final ArrayList handlers = new ArrayList<>(); + final LinkedBlockingQueue tasks = new LinkedBlockingQueue<>(); + final LinkedBlockingQueue asynchronous = new LinkedBlockingQueue<>(); + Fixture() { + when(plugin.getAdminVoteCommand()).thenReturn(handlers); + when(plugin.getVotingNPCManager()).thenReturn(manager); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(manager.available()).thenReturn(true); + doAnswer(inv -> { tasks.add(inv.getArgument(1)); return null; }) + .when(scheduler).runTask(eq(plugin), any(Runnable.class), any(Entity.class)); + doAnswer(inv -> { tasks.add(inv.getArgument(1)); return null; }) + .when(scheduler).runTask(eq(plugin), any(Runnable.class)); + doAnswer(inv -> { asynchronous.add(inv.getArgument(1)); return null; }) + .when(scheduler).runTaskAsynchronously(eq(plugin), any(Runnable.class)); + VotingNPCCommands.register(plugin); + } + Player player() { + Player player = mock(Player.class); + when(player.isOnline()).thenReturn(true); + when(player.hasPermission("VotingPlugin.Commands.AdminVote.NPC")).thenReturn(true); + World world = mock(World.class); when(world.getUID()).thenReturn(new UUID(1, 0)); + when(player.getLocation()).thenReturn(new Location(world, 4, 64, 4)); + return player; + } + CommandHandler command(String action, int arguments) { + return handlers.stream().filter(value -> value.getArgs()[1].equals(action) + && value.getArgs().length == arguments).findFirst().orElseThrow(); + } + } + + @Test void usesOnlyDedicatedNpcPatternsAndExplicitAdminPermission() { + var f = new Fixture(); + assertEquals(List.of(List.of("NPC", "Create", "(string)"), List.of("NPC", "Create", "(string)", "(list)"), + List.of("NPC", "List"), List.of("NPC", "Inspect", "(npcid)"), List.of("NPC", "Remove", "(npcid)")), + f.handlers.stream().map(value -> List.of(value.getArgs())).toList()); + for (var handler : f.handlers) assertEquals("VotingPlugin.Commands.AdminVote.NPC|VotingPlugin.Admin", handler.getPerm()); + } + + @Test void capturesPlayerStateOnlyOnOwnerAndSuccessOnlyAfterDurableCompletion() { + var f = new Fixture(); Player player = f.player(); + var completion = new CompletableFuture(); + when(f.manager.create(any())).thenReturn(completion); + f.command("Create", 4).execute(player, new String[] { "NPC", "Create", "voting", "Vote", "here" }); + verify(player, never()).getLocation(); verify(f.manager, never()).create(any()); + f.tasks.remove().run(); + var definition = ArgumentCaptor.forClass(VotingNPCDefinition.class); verify(f.manager).create(definition.capture()); + assertEquals("Vote here", definition.getValue().name()); assertEquals("voting", definition.getValue().id()); + verify(player, never()).sendMessage(anyString()); + completion.complete(definition.getValue()); + verify(player, never()).sendMessage(anyString()); + f.tasks.remove().run(); + verify(player).sendMessage(contains("Saved voting NPC 'voting'")); + } + + @Test void delayedPermissionRevocationAndDisabledFeaturePreventMutation() { + var f = new Fixture(); Player player = f.player(); + var handler = f.command("Create", 3); + handler.execute(player, new String[] { "NPC", "Create", "voting" }); + when(player.hasPermission("VotingPlugin.Commands.AdminVote.NPC")).thenReturn(false); + f.tasks.remove().run(); verify(f.manager, never()).create(any()); + when(player.hasPermission("VotingPlugin.Commands.AdminVote.NPC")).thenReturn(true); + when(f.manager.available()).thenReturn(false); + handler.execute(player, new String[] { "NPC", "Create", "voting" }); f.tasks.remove().run(); + verify(f.manager, never()).create(any()); verify(player).sendMessage(contains("VotingNPCs.Enabled")); + } + + @Test void rejectedStorageDoesNotSendSuccessAndDoesNotLeakRawFailure() { + var f = new Fixture(); Player player = f.player(); + when(f.manager.remove("voting")).thenReturn(CompletableFuture.failedFuture(new IllegalStateException("Synthetic private diagnostic"))); + f.command("Remove", 3).execute(player, new String[] { "NPC", "Remove", "voting" }); + f.tasks.remove().run(); f.tasks.remove().run(); + verify(player).sendMessage(contains("was not confirmed")); + verify(player, never()).sendMessage(contains("Synthetic private")); + verify(player, never()).sendMessage(startsWith("Removed saved")); + } + + @Test void invalidDefinitionAndConsoleCreationDoNotMutate() { + var f = new Fixture(); Player player = f.player(); + f.command("Create", 3).execute(player, new String[] { "NPC", "Create", "bad id" }); f.tasks.remove().run(); + verify(f.manager, never()).create(any()); verify(player).sendMessage(contains("lowercase ID")); + CommandSender console = mock(CommandSender.class); when(console.hasPermission("VotingPlugin.Admin")).thenReturn(true); + f.command("Create", 3).execute(console, new String[] { "NPC", "Create", "voting" }); f.tasks.remove().run(); + verify(console).sendMessage("Create a voting NPC from in game."); verify(f.manager, never()).create(any()); + } + + @Test void bothCreationFormsMatchRealCommandDispatchWithStandardFrameworkPlaceholders() { + var tabs = new com.bencodez.simpleapi.command.TabCompleteHandler(); + // These are the standard placeholders AdvancedCore registers before commands. + tabs.addTabCompleteOption("(String)", new ArrayList<>()); + tabs.addTabCompleteOption("(List)", new ArrayList<>()); + try (var tabSingleton = mockStatic(com.bencodez.simpleapi.command.TabCompleteHandler.class)) { + tabSingleton.when(com.bencodez.simpleapi.command.TabCompleteHandler::getInstance).thenReturn(tabs); + var f = new Fixture(); Player player = f.player(); + when(f.manager.create(any())).thenReturn(new CompletableFuture<>()); + assertTrue(f.command("Create", 3).runCommand(player, new String[] { "npc", "create", "new-voting" })); + f.asynchronous.remove().run(); f.tasks.remove().run(); + assertTrue(f.command("Create", 4).runCommand(player, new String[] { "npc", "create", "another", "Vote", "here" })); + f.asynchronous.remove().run(); f.tasks.remove().run(); + var values = ArgumentCaptor.forClass(VotingNPCDefinition.class); verify(f.manager, times(2)).create(values.capture()); + assertEquals(List.of("new-voting", "another"), values.getAllValues().stream().map(VotingNPCDefinition::id).toList()); + } + } +} diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java new file mode 100644 index 0000000000..19d6fad11f --- /dev/null +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java @@ -0,0 +1,324 @@ +package com.bencodez.votingplugin.npc; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; +import static org.mockito.AdditionalMatchers.aryEq; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.logging.Logger; + +import org.bukkit.Bukkit; +import org.bukkit.Chunk; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Entity; +import org.bukkit.entity.Player; +import org.bukkit.entity.Villager; +import org.bukkit.event.entity.EntityDamageEvent; +import org.bukkit.event.player.PlayerInteractEntityEvent; +import org.bukkit.event.world.ChunkLoadEvent; +import org.bukkit.event.world.ChunkUnloadEvent; +import org.bukkit.event.world.WorldLoadEvent; +import org.bukkit.event.world.WorldUnloadEvent; +import org.bukkit.inventory.EquipmentSlot; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.bencodez.simpleapi.scheduler.BukkitScheduler; +import com.bencodez.votingplugin.VotingPluginMain; + +class VotingNPCManagerTest { + @TempDir Path directory; + private static final UUID WORLD = new UUID(1, 0); + + private static VotingNPCDefinition definition(int identity) { + return new VotingNPCDefinition("voting", new UUID(0, identity), WORLD, 8, 64, 8, 0, 0, "&aVote"); + } + + private static T await(CompletionStage stage) throws Exception { + return stage.toCompletableFuture().get(5, TimeUnit.SECONDS); + } + + private static class Scheduler extends VotingNPCScheduler { + final LinkedBlockingQueue global = new LinkedBlockingQueue<>(); + final LinkedBlockingQueue regions = new LinkedBlockingQueue<>(); + final LinkedBlockingQueue entities = new LinkedBlockingQueue<>(); + String owner = "none"; + boolean rejectCleanup; + Scheduler(VotingPluginMain plugin) { super(plugin); } + @Override boolean owns(Location value) { return owner.equals("region"); } + @Override boolean owns(Entity value) { return owner.equals("entity"); } + @Override void global(Runnable action, Consumer failed) { global.add(action); } + @Override void region(Location anchor, Runnable action, Consumer failed) { regions.add(action); } + @Override void entity(Entity entity, Runnable action, Runnable retired, Consumer failed) { + if (rejectCleanup) failed.accept(new IllegalStateException("Synthetic rejection")); + else entities.add(action); + } + void runNext(LinkedBlockingQueue queue, String phase) throws Exception { + Runnable action = queue.poll(5, TimeUnit.SECONDS); + assertNotNull(action, "Expected scheduled " + phase + " work"); + owner = phase; + try { action.run(); } finally { owner = "none"; } + } + } + + @Test void duplicateChunkNotificationsDoNotReconstructAnotherEntityAndOrdinaryEntitiesAreIgnored() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); + List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + var manager = new VotingNPCManager(plugin, scheduler, registry, player -> fail("Unexpected menu")); + try { + manager.configure(true); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + Chunk chunk = chunk(world); + manager.chunkLoad(new ChunkLoadEvent(chunk, false)); + manager.chunkLoad(new ChunkLoadEvent(chunk, false)); + assertEquals(1, spawned.size()); assertEquals(1, manager.runtimeCount()); + verify(spawned.getFirst()).setPersistent(false); + verify(spawned.getFirst()).setAI(false); + verify(spawned.getFirst()).setCustomName("\u00a7aVote"); + Player player = mock(Player.class); + Villager unrelated = mock(Villager.class); when(unrelated.getUniqueId()).thenReturn(new UUID(0, 99)); + var interact = new PlayerInteractEntityEvent(player, unrelated, EquipmentSlot.HAND); + manager.interact(interact); assertFalse(interact.isCancelled()); + var damage = mock(EntityDamageEvent.class); when(damage.getEntity()).thenReturn(unrelated); + manager.damage(damage); verify(damage, never()).setCancelled(anyBoolean()); + var protectedDamage = mock(EntityDamageEvent.class); when(protectedDamage.getEntity()).thenReturn(spawned.getFirst()); + manager.damage(protectedDamage); verify(protectedDamage).setCancelled(true); + var witch = mock(org.bukkit.entity.Witch.class); + var transform = new org.bukkit.event.entity.EntityTransformEvent(spawned.getFirst(), List.of(witch), + org.bukkit.event.entity.EntityTransformEvent.TransformReason.LIGHTNING); + var unrelatedTransform = new org.bukkit.event.entity.EntityTransformEvent(unrelated, List.of(witch), + org.bukkit.event.entity.EntityTransformEvent.TransformReason.LIGHTNING); + // Discover actual listener methods, as event registration does; + // absence of a transformation handler must fail this regression. + for (var method : VotingNPCManager.class.getDeclaredMethods()) { + if (method.isAnnotationPresent(org.bukkit.event.EventHandler.class) && method.getParameterCount() == 1 + && method.getParameterTypes()[0].isInstance(transform)) { + method.invoke(manager, transform); method.invoke(manager, unrelatedTransform); + } + } + assertTrue(transform.isCancelled(), "lightning must not replace a tracked villager with an untracked entity"); + assertFalse(unrelatedTransform.isCancelled()); + } finally { + manager.close(); scheduler.runNext(scheduler.entities, "entity"); + assertEquals(0, manager.runtimeCount()); assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + + @Test void successorWaitsForExactPredecessorCleanupAndCleanupRejectionRetainsOwnership() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + scheduler.rejectCleanup = true; + assertTrue(await(manager.remove("voting"))); + await(manager.create(definition(2))); + // Process both durable mutation notifications while old removal is rejected. + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.global, "global"); + assertEquals(1, spawned.size()); assertEquals(1, manager.runtimeCount()); + verify(spawned.getFirst(), never()).remove(); + scheduler.rejectCleanup = false; + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + scheduler.runNext(scheduler.entities, "entity"); + verify(spawned.getFirst()).remove(); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + assertEquals(2, spawned.size()); assertEquals(1, manager.runtimeCount()); + } finally { + scheduler.rejectCleanup = false; manager.close(); + if (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + + @Test void unloadedChunksNeverSpawnOrLoadAndLaterLoadReconstructsOnce() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + assertTrue(spawned.isEmpty()); assertEquals(0, manager.runtimeCount()); + verify(world, never()).getChunkAt(anyInt(), anyInt()); verify(world, never()).loadChunk(anyInt(), anyInt()); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + manager.chunkLoad(new ChunkLoadEvent(chunk(world), false)); scheduler.runNext(scheduler.regions, "region"); + assertEquals(1, spawned.size()); + manager.chunkUnload(new ChunkUnloadEvent(chunk(world))); scheduler.runNext(scheduler.entities, "entity"); + assertTrue(scheduler.global.isEmpty(), "unload must not immediately retry reconstruction"); + assertTrue(scheduler.regions.isEmpty()); + manager.chunkLoad(new ChunkLoadEvent(chunk(world), false)); scheduler.runNext(scheduler.regions, "region"); + assertEquals(2, spawned.size()); + } finally { manager.close(); scheduler.runNext(scheduler.entities, "entity"); assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); } + } + } + + @Test void partialSpawnFailureRemovesOnlyTheCapturedEntityAndCloseInvalidatesQueuedSpawns() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + // Close before a deferred creation runs: no entity may be created. + manager.configure(false); scheduler.runNext(scheduler.regions, "region"); + assertTrue(spawned.isEmpty()); assertEquals(0, manager.runtimeCount()); + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + doAnswer(invocation -> { + Villager entity = mock(Villager.class); when(entity.getUniqueId()).thenReturn(new UUID(0, 17)); + doThrow(new IllegalStateException("Synthetic entity property failure")).when(entity).setAI(false); + spawned.add(entity); + Consumer initializer = invocation.getArgument(2); initializer.accept(entity); return entity; + }).when(world).spawn(any(Location.class), eq(Villager.class), any(Consumer.class)); + scheduler.runNext(scheduler.regions, "region"); + assertEquals(1, manager.runtimeCount()); + scheduler.runNext(scheduler.entities, "entity"); + verify(spawned.getFirst()).remove(); assertEquals(0, manager.runtimeCount()); + assertTrue(scheduler.global.isEmpty(), "failed spawn must not cause an automatic retry loop"); + } finally { manager.close(); assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); } + } + } + + @Test void disabledFeatureDoesNotReadStorageOrRegisterAnyRuntimeWork() { + var plugin = mock(VotingPluginMain.class); + var scheduler = new Scheduler(plugin); + var manager = new VotingNPCManager(plugin, scheduler, null, ignored -> fail("Disabled menu")); + manager.configure(false); + assertFalse(manager.available()); assertEquals(0, manager.runtimeCount()); + assertTrue(scheduler.global.isEmpty()); assertTrue(scheduler.regions.isEmpty()); + verify(plugin, never()).getDataFolder(); + manager.close(); manager.close(); + } + + @Test void worldUnloadPreservesDefinitionsAndAWorldLoadAfterCleanupRecreatesOnce() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + manager.worldUnload(new WorldUnloadEvent(world)); + // A load before delayed cleanup must record demand, not spawn a duplicate. + manager.worldLoad(new WorldLoadEvent(world)); scheduler.runNext(scheduler.global, "global"); + assertEquals(1, spawned.size()); + scheduler.runNext(scheduler.entities, "entity"); scheduler.runNext(scheduler.global, "global"); + scheduler.runNext(scheduler.regions, "region"); + assertEquals(2, spawned.size()); assertEquals(List.of(definition(1)), manager.definitions()); + } finally { manager.close(); scheduler.runNext(scheduler.entities, "entity"); assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); } + } + } + + @Test void cancelledAndOffhandClicksDoNotOpenAndRemovalInvalidatesAQueuedPlayerCallback() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + var bukkitScheduler = mock(BukkitScheduler.class); when(plugin.getBukkitScheduler()).thenReturn(bukkitScheduler); + var playerCallbacks = new LinkedBlockingQueue(); + doAnswer(invocation -> { playerCallbacks.add(invocation.getArgument(1)); return null; }) + .when(bukkitScheduler).runTask(eq(plugin), any(Runnable.class), any(Entity.class)); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + List opened = new ArrayList<>(); + var manager = new VotingNPCManager(plugin, scheduler, registry, opened::add); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + Player player = mock(Player.class); when(player.getUniqueId()).thenReturn(new UUID(0, 50)); + when(player.isOnline()).thenReturn(true); when(player.getWorld()).thenReturn(world); + when(player.getLocation()).thenReturn(new Location(world, 8, 64, 9)); + var cancelled = new PlayerInteractEntityEvent(player, spawned.getFirst(), EquipmentSlot.HAND); cancelled.setCancelled(true); + manager.interact(cancelled); + manager.interact(new PlayerInteractEntityEvent(player, spawned.getFirst(), EquipmentSlot.OFF_HAND)); + assertTrue(playerCallbacks.isEmpty()); + var click = new PlayerInteractEntityEvent(player, spawned.getFirst(), EquipmentSlot.HAND); + manager.interact(click); assertTrue(click.isCancelled()); + assertEquals(1, playerCallbacks.size()); + playerCallbacks.remove().run(); + assertEquals(List.of(player), opened, "healthy main-hand selection opens on the player callback"); + Player second = mock(Player.class); when(second.getUniqueId()).thenReturn(new UUID(0, 51)); + when(second.isOnline()).thenReturn(true); when(second.getWorld()).thenReturn(world); + when(second.getLocation()).thenReturn(new Location(world, 8, 64, 9)); + manager.interact(new PlayerInteractEntityEvent(second, spawned.getFirst(), EquipmentSlot.HAND)); + playerCallbacks.remove().run(); + assertEquals(List.of(player, second), opened, "one player's selection must not block another player"); + Player third = mock(Player.class); when(third.getUniqueId()).thenReturn(new UUID(0, 52)); + when(third.isOnline()).thenReturn(true); when(third.getWorld()).thenReturn(world); + when(third.getLocation()).thenReturn(new Location(world, 8, 64, 9)); + manager.interact(new PlayerInteractEntityEvent(third, spawned.getFirst(), EquipmentSlot.HAND)); + await(manager.remove("voting")); + playerCallbacks.remove().run(); assertEquals(List.of(player, second), opened); + } finally { manager.close(); scheduler.runNext(scheduler.entities, "entity"); assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); } + } + } + + @Test void productionMenuEntryDelegatesToTheExistingUrlCommandRatherThanAnotherMenuOrRewardPath() { + var plugin = mock(VotingPluginMain.class); + var unrelated = mock(com.bencodez.advancedcore.api.command.CommandHandler.class); + var url = mock(com.bencodez.advancedcore.api.command.CommandHandler.class); + var later = mock(com.bencodez.advancedcore.api.command.CommandHandler.class); + var player = mock(Player.class); + when(plugin.getVoteCommand()).thenReturn(new ArrayList<>(List.of(unrelated, url, later))); + when(url.runCommand(eq(player), aryEq(new String[] { "URL" }))).thenReturn(true); + VotingNPCManager.openVotingSites(plugin, player); + verify(unrelated).runCommand(eq(player), aryEq(new String[] { "URL" })); + verify(url).runCommand(eq(player), aryEq(new String[] { "URL" })); + verifyNoInteractions(later); + verify(player, never()).performCommand(anyString()); + } + + private static Chunk chunk(World world) { + Chunk chunk = mock(Chunk.class); when(chunk.getWorld()).thenReturn(world); when(chunk.getX()).thenReturn(0); when(chunk.getZ()).thenReturn(0); return chunk; + } + + @SuppressWarnings("unchecked") + private static List installSpawner(World world, Scheduler scheduler) { + List spawned = new ArrayList<>(); + doAnswer(invocation -> { + assertEquals("region", scheduler.owner); + Villager entity = mock(Villager.class); when(entity.getUniqueId()).thenReturn(new UUID(2, spawned.size() + 1)); + when(entity.isValid()).thenReturn(true); + Consumer initializer = invocation.getArgument(2); initializer.accept(entity); + doAnswer(removal -> { assertEquals("entity", scheduler.owner); return null; }).when(entity).remove(); + spawned.add(entity); return entity; + }).when(world).spawn(any(Location.class), eq(Villager.class), any(Consumer.class)); + return spawned; + } +} diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java new file mode 100644 index 0000000000..28bee20064 --- /dev/null +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java @@ -0,0 +1,214 @@ +package com.bencodez.votingplugin.npc; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class VotingNPCRegistryTest { + @TempDir Path directory; + + private VotingNPCDefinition definition(int id) { + return new VotingNPCDefinition("npc-" + id, new UUID(0, id), new UUID(1, 0), + 0.5, 64, 0.5, 0, 0, "Vote"); + } + + private static T await(CompletionStage stage) throws Exception { + return stage.toCompletableFuture().get(5, TimeUnit.SECONDS); + } + + private static void close(VotingNPCRegistry registry) throws InterruptedException { + registry.close(); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + + private static void waitOn(CountDownLatch latch) throws IOException { + try { + if (!latch.await(5, TimeUnit.SECONDS)) throw new IOException("Synthetic worker latch timed out"); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw new IOException("Synthetic worker interrupted", interrupted); + } + } + + @Test void orderedDefinitionsAreDurableAndDuplicateRequestsDoNotOverwrite() throws Exception { + Path file = directory.resolve("npcs.json"); + var registry = new VotingNPCRegistry(file); + try { + assertEquals(List.of(), await(registry.initialLoad())); + var first = registry.create(definition(1)); + var duplicate = registry.create(definition(1)); + var second = registry.create(definition(2)); + assertEquals(definition(1), await(first)); + assertInstanceOf(IllegalArgumentException.class, + assertThrows(ExecutionException.class, () -> await(duplicate)).getCause()); + await(second); + assertEquals(List.of(definition(1), definition(2)), new VotingNPCStore(file).load()); + assertTrue(await(registry.remove("npc-1"))); + assertFalse(await(registry.remove("npc-1"))); + } finally { close(registry); } + var restarted = new VotingNPCRegistry(file); + try { assertEquals(List.of(definition(2)), await(restarted.initialLoad())); } + finally { close(restarted); } + } + + @Test void failedReloadRetainsLastSnapshotButCannotPublishUntilRepaired() throws Exception { + Path file = directory.resolve("npcs.json"); + var registry = new VotingNPCRegistry(file); + try { + await(registry.initialLoad()); await(registry.create(definition(1))); + byte[] healthy = Files.readAllBytes(file); + Files.writeString(file, "{invalid"); + assertThrows(ExecutionException.class, () -> await(registry.reload())); + assertFalse(registry.available()); + assertEquals(List.of(definition(1)), registry.snapshot()); + assertThrows(ExecutionException.class, () -> await(registry.create(definition(2)))); + assertEquals("{invalid", Files.readString(file)); + Files.write(file, healthy); + assertEquals(List.of(definition(1)), await(registry.reload())); + await(registry.create(definition(2))); + } finally { close(registry); } + } + + @Test void invalidInitialFileDoesNotAppearReadyOrGetReplaced() throws Exception { + Path file = directory.resolve("npcs.json"); Files.writeString(file, "{invalid"); + var registry = new VotingNPCRegistry(file); + try { + assertThrows(ExecutionException.class, () -> await(registry.initialLoad())); + assertFalse(registry.available()); + assertThrows(ExecutionException.class, () -> await(registry.create(definition(1)))); + assertEquals("{invalid", Files.readString(file)); + } finally { close(registry); } + } + + @Test void closeReturnsWhileAcceptedWriteDrainsAndKeepsExclusiveFileOwnership() throws Exception { + Path file = directory.resolve("npcs.json"); + CountDownLatch entered = new CountDownLatch(1), release = new CountDownLatch(1); + var store = new VotingNPCStore(file) { + @Override synchronized void save(List values) throws IOException { + entered.countDown(); waitOn(release); super.save(values); + } + }; + var first = new VotingNPCRegistry(file, store); + VotingNPCRegistry replacement = null; + try { + await(first.initialLoad()); + var writing = first.create(definition(1)); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + first.close(); first.close(); + assertFalse(first.awaitClosed(0, TimeUnit.SECONDS)); + assertThrows(ExecutionException.class, () -> await(first.remove("npc-1"))); + replacement = new VotingNPCRegistry(file); + var next = replacement; + assertThrows(ExecutionException.class, () -> await(next.initialLoad())); + assertFalse(replacement.available()); + release.countDown(); await(writing); + assertTrue(first.awaitClosed(5, TimeUnit.SECONDS)); + assertEquals(List.of(definition(1)), await(replacement.reload())); + } finally { + release.countDown(); close(first); + if (replacement != null) close(replacement); + } + } + + @Test void saturatedReloadIsRejectedWithoutDisablingTheHealthyOwner() throws Exception { + Path file = directory.resolve("npcs.json"); + CountDownLatch entered = new CountDownLatch(1), release = new CountDownLatch(1); + var store = new VotingNPCStore(file) { + @Override synchronized void save(List values) throws IOException { + entered.countDown(); waitOn(release); super.save(values); + } + }; + var registry = new VotingNPCRegistry(file, store); + try { + await(registry.initialLoad()); + var writing = registry.create(definition(1)); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + var queued = new java.util.ArrayList>(); + for (int i = 0; i < 128; i++) queued.add(registry.remove("missing")); + assertThrows(ExecutionException.class, () -> await(registry.reload())); + assertTrue(registry.available(), "rejected reload must leave existing readiness intact"); + release.countDown(); await(writing); + for (var entry : queued) assertFalse(await(entry)); + await(registry.create(definition(2))); + } finally { release.countDown(); close(registry); } + } + + @Test void failedWriteFencesAlreadyQueuedMutationsUntilCheckedReload() throws Exception { + Path file = directory.resolve("npcs.json"); + CountDownLatch entered = new CountDownLatch(1), release = new CountDownLatch(1); + AtomicBoolean fail = new AtomicBoolean(true); + var store = new VotingNPCStore(file) { + @Override synchronized void save(List values) throws IOException { + if (fail.getAndSet(false)) { + entered.countDown(); waitOn(release); + super.save(values); + throw new IOException("Synthetic failure after visible write"); + } + super.save(values); + } + }; + var registry = new VotingNPCRegistry(file, store); + try { + await(registry.initialLoad()); + var first = registry.create(definition(1)); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + var second = registry.create(definition(2)); + release.countDown(); + assertThrows(ExecutionException.class, () -> await(first)); + assertThrows(ExecutionException.class, () -> await(second)); + assertFalse(registry.available()); + assertEquals(List.of(), registry.snapshot()); + assertEquals(List.of(definition(1)), new VotingNPCStore(file).load()); + assertEquals(List.of(definition(1)), await(registry.reload())); + await(registry.create(definition(2))); + assertEquals(List.of(definition(1), definition(2)), registry.snapshot()); + } finally { release.countDown(); close(registry); } + } + + @Test void completionCallbackMayCloseWithoutDeadlockingOrInterruptingCommittedWork() throws Exception { + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); + try { + await(registry.initialLoad()); + CompletableFuture callback = registry.create(definition(1)).thenAccept(value -> registry.close()) + .toCompletableFuture(); + await(callback); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + assertEquals(List.of(definition(1)), new VotingNPCStore(directory.resolve("npcs.json")).load()); + } finally { close(registry); } + } + + @Test void capacityAndIdentityRejectionsLeaveHealthyStorageAvailable() throws Exception { + Path file = directory.resolve("npcs.json"); + var seed = new VotingNPCStore(file); seed.load(); + seed.save(java.util.stream.IntStream.rangeClosed(1, 63).mapToObj(this::definition).toList()); + var registry = new VotingNPCRegistry(file); + try { + await(registry.initialLoad()); + byte[] original = Files.readAllBytes(file); + var duplicateIdentity = new VotingNPCDefinition("another", definition(1).identity(), definition(1).world(), + 0, 64, 0, 0, 0, "Vote"); + assertThrows(ExecutionException.class, () -> await(registry.create(duplicateIdentity))); + assertTrue(registry.available(), "invalid new identity must not disable existing NPC storage"); + assertArrayEquals(original, Files.readAllBytes(file)); + await(registry.create(definition(64))); + byte[] atLimit = Files.readAllBytes(file); + assertThrows(ExecutionException.class, () -> await(registry.create(definition(65)))); + assertTrue(registry.available(), "one extra creation must not disable 64 existing NPCs"); + assertEquals(64, registry.snapshot().size()); + assertArrayEquals(atLimit, Files.readAllBytes(file)); + } finally { close(registry); } + } +} diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCSchedulerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCSchedulerTest.java new file mode 100644 index 0000000000..c469430597 --- /dev/null +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCSchedulerTest.java @@ -0,0 +1,78 @@ +package com.bencodez.votingplugin.npc; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; + +import org.bukkit.Bukkit; +import org.bukkit.Location; +import org.bukkit.entity.Entity; +import org.junit.jupiter.api.Test; + +import com.bencodez.votingplugin.VotingPluginMain; + +class VotingNPCSchedulerTest { + @Test void alreadyOwnedCleanupRunsInlineEvenDuringDisable() { + var plugin = mock(VotingPluginMain.class); + try (var bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::isPrimaryThread).thenReturn(true); + var scheduler = new VotingNPCScheduler(plugin); var ran = new AtomicInteger(); + scheduler.entity(mock(Entity.class), ran::incrementAndGet, () -> fail("retired"), error -> fail(error)); + assertEquals(1, ran.get()); verify(plugin, never()).getBukkitScheduler(); + } + } + + @Test void deferredRegionActionUsesTheLocationSchedulerAndReportsFailure() { + var plugin = mock(VotingPluginMain.class, RETURNS_DEEP_STUBS); + try (var bukkit = mockStatic(Bukkit.class)) { + var scheduler = new VotingNPCScheduler(plugin); Location anchor = mock(Location.class); + var completion = new CompletableFuture(); + var callback = new AtomicReference(); + when(plugin.getBukkitScheduler().getFoliaLib().getImpl().runAtLocation(eq(anchor), any())) + .thenAnswer(inv -> { Consumer action = inv.getArgument(1); callback.set(() -> action.accept(null)); return completion; }); + var ran = new AtomicInteger(); List failures = new ArrayList<>(); + scheduler.region(anchor, ran::incrementAndGet, failures::add); + assertEquals(0, ran.get()); callback.get().run(); assertEquals(1, ran.get()); + completion.completeExceptionally(new IllegalStateException("Synthetic scheduler failure")); + assertEquals(1, failures.size()); + } + } + + @Test @SuppressWarnings({"unchecked", "rawtypes"}) + void schedulerRetirementDoesNotPretendThatAnEntityWasRemoved() throws Exception { + var plugin = mock(VotingPluginMain.class, RETURNS_DEEP_STUBS); + try (var bukkit = mockStatic(Bukkit.class)) { + var scheduler = new VotingNPCScheduler(plugin); Entity entity = mock(Entity.class); + CompletableFuture completion = new CompletableFuture(); + when(plugin.getBukkitScheduler().getFoliaLib().getImpl().runAtEntityWithFallback(eq(entity), any(), any())) + .thenReturn(completion); + var ran = new AtomicInteger(); var retired = new AtomicInteger(); List failures = new ArrayList<>(); + scheduler.entity(entity, ran::incrementAndGet, retired::incrementAndGet, failures::add); + completion.complete(Enum.valueOf((Class) Class.forName("com.bencodez.simpleapi.folialib.enums.EntityTaskResult"), "SCHEDULER_RETIRED")); + assertEquals(0, ran.get()); assertEquals(0, retired.get()); assertEquals(1, failures.size()); + } + } + + @Test @SuppressWarnings({"unchecked", "rawtypes"}) + void entityRetirementFallbackAndFutureCompletionAreHandledOnce() throws Exception { + var plugin = mock(VotingPluginMain.class, RETURNS_DEEP_STUBS); + try (var bukkit = mockStatic(Bukkit.class)) { + var scheduler = new VotingNPCScheduler(plugin); Entity entity = mock(Entity.class); + CompletableFuture completion = new CompletableFuture(); var fallback = new AtomicReference(); + when(plugin.getBukkitScheduler().getFoliaLib().getImpl().runAtEntityWithFallback(eq(entity), any(), any())) + .thenAnswer(inv -> { fallback.set(inv.getArgument(2)); return completion; }); + var retired = new AtomicInteger(); + scheduler.entity(entity, () -> fail("Entity already retired"), retired::incrementAndGet, error -> fail(error)); + fallback.get().run(); + completion.complete(Enum.valueOf((Class) Class.forName("com.bencodez.simpleapi.folialib.enums.EntityTaskResult"), "ENTITY_RETIRED")); + assertEquals(1, retired.get()); + } + } +} diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java new file mode 100644 index 0000000000..9f37206a10 --- /dev/null +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java @@ -0,0 +1,141 @@ +package com.bencodez.votingplugin.npc; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class VotingNPCStoreTest { + @TempDir Path directory; + + private VotingNPCDefinition definition(int id) { + return new VotingNPCDefinition("npc-" + id, new UUID(0, id), new UUID(1, 0), + -0.5, 64, 16.5, 90, 0, "Vote"); + } + + @Test void roundTripPreservesStableIdentitiesAndPositionAcrossRestart() throws Exception { + Path file = directory.resolve("npcs.json"); + var store = new VotingNPCStore(file); + assertEquals(List.of(), store.load()); + store.save(List.of(definition(1), definition(2))); + assertEquals(List.of(definition(1), definition(2)), new VotingNPCStore(file).load()); + assertEquals(-1, definition(1).chunkX()); + assertEquals(1, definition(1).chunkZ()); + } + + @Test void missingInitialReadDoesNotCreateFilesAndUninitializedStoreCannotOverwrite() throws Exception { + Path file = directory.resolve("npcs.json"); + var store = new VotingNPCStore(file); + assertThrows(IOException.class, () -> store.save(List.of(definition(1)))); + assertFalse(Files.exists(file)); + assertTrue(store.load().isEmpty()); + assertFalse(Files.exists(file)); + } + + @Test void invalidLoadKeepsOriginalBytesAndBlocksMutations() throws Exception { + Path file = directory.resolve("npcs.json"); + byte[] original = "{ damaged definitions".getBytes(StandardCharsets.UTF_8); + Files.write(file, original); + var store = new VotingNPCStore(file); + assertThrows(IOException.class, store::load); + assertThrows(IOException.class, () -> store.save(List.of())); + assertArrayEquals(original, Files.readAllBytes(file)); + } + + @Test void duplicateIdentitiesAndCapacityRejectionDoNotChangeDurableDefinitions() throws Exception { + Path file = directory.resolve("npcs.json"); + var store = new VotingNPCStore(file); store.load(); store.save(List.of(definition(1))); + byte[] before = Files.readAllBytes(file); + assertThrows(IOException.class, () -> store.save(List.of(definition(1), definition(1)))); + var sameIdentity = new VotingNPCDefinition("another", definition(1).identity(), definition(1).world(), + 0, 64, 0, 0, 0, "Vote"); + assertThrows(IOException.class, () -> store.save(List.of(definition(1), sameIdentity))); + assertThrows(IOException.class, () -> store.save(IntStream.rangeClosed(1, 65).mapToObj(this::definition).toList())); + assertArrayEquals(before, Files.readAllBytes(file)); + } + + @Test void publicationFailureFencesOldStateUntilCheckedReload() throws Exception { + Path file = directory.resolve("npcs.json"); + AtomicBoolean fail = new AtomicBoolean(); + var store = new VotingNPCStore(file) { + @Override void forceDirectory(Path parent) throws IOException { + if (fail.get()) throw new IOException("Synthetic directory sync failure"); + super.forceDirectory(parent); + } + }; + store.load(); store.save(List.of(definition(1))); + fail.set(true); + assertThrows(IOException.class, () -> store.save(List.of(definition(1), definition(2)))); + byte[] uncertain = Files.readAllBytes(file); + fail.set(false); + assertThrows(IOException.class, () -> store.save(List.of(definition(1)))); + assertArrayEquals(uncertain, Files.readAllBytes(file)); + assertEquals(List.of(definition(1), definition(2)), store.load()); + store.save(List.of(definition(2))); + assertEquals(List.of(definition(2)), new VotingNPCStore(file).load()); + } + + @Test void missingUncertainFileDoesNotBecomeAnEmptySuccessfulRecovery() throws Exception { + Path file = directory.resolve("npcs.json"); + var store = new VotingNPCStore(file) { + @Override void forceDirectory(Path ignored) throws IOException { throw new IOException("Synthetic failure"); } + }; + store.load(); assertThrows(IOException.class, () -> store.save(List.of(definition(1)))); + Files.delete(file); + assertThrows(IOException.class, store::load); + assertThrows(IOException.class, () -> store.save(List.of())); + assertFalse(Files.exists(file)); + } + + @Test void strictFieldsAndTypesRejectMalformedDocumentsIndividually() throws Exception { + Path file = directory.resolve("npcs.json"); + var store = new VotingNPCStore(file); store.load(); store.save(List.of(definition(1))); + String healthy = Files.readString(file); + for (String damaged : List.of( + healthy.replace("\"version\": 1", "\"version\": 1, \"version\": 1"), + healthy.replace("\"version\": 1", "\"version\": \"1\""), + healthy.replace("\"version\": 1", "\"version\": 1.0"), + healthy.replace("\"id\": \"npc-1\"", "\"id\": \"npc-1\", \"id\": \"other\""), + healthy.replace("\"x\": -0.5", "\"x\": \"-0.5\""), + healthy.replace("\"name\": \"Vote\"", "\"unknown\": \"Vote\""), + healthy.replace("\"name\": \"Vote\"", "\"name\": null"), + healthy.replace("\"yaw\": 90.0", "\"yaw\": 1e100"), + healthy + "{}")) { + Files.writeString(file, damaged); + assertThrows(IOException.class, () -> new VotingNPCStore(file).load(), damaged); + assertEquals(damaged, Files.readString(file)); + } + } + + @Test void invalidUtf8OversizeAndSymlinkCannotBeLoadedOrOverwritten() throws Exception { + Path file = directory.resolve("npcs.json"); + Files.write(file, new byte[] {(byte) 0xc3, (byte) 0x28}); + assertThrows(IOException.class, () -> new VotingNPCStore(file).load()); + Files.write(file, new byte[128 * 1024 + 1]); + assertThrows(IOException.class, () -> new VotingNPCStore(file).load()); + Files.delete(file); + Path target = directory.resolve("unrelated.json"); + Files.writeString(target, "unrelated"); + Files.createSymbolicLink(file, target); + assertThrows(IOException.class, () -> new VotingNPCStore(file).load()); + assertEquals("unrelated", Files.readString(target)); + } + + @Test void invalidDefinitionsAreRejectedBeforePublication() { + assertThrows(IllegalArgumentException.class, () -> new VotingNPCDefinition("bad id", new UUID(0, 1), + new UUID(1, 0), 0, 64, 0, 0, 0, "Vote")); + assertThrows(IllegalArgumentException.class, () -> new VotingNPCDefinition("valid", new UUID(0, 1), + new UUID(1, 0), Double.NaN, 64, 0, 0, 0, "Vote")); + assertThrows(IllegalArgumentException.class, () -> new VotingNPCDefinition("valid", new UUID(0, 1), + new UUID(1, 0), 0, 64, 0, 0, 0, "Vote\nNew line")); + } +} diff --git a/docs/voting-npcs.md b/docs/voting-npcs.md new file mode 100644 index 0000000000..842ef785f9 --- /dev/null +++ b/docs/voting-npcs.md @@ -0,0 +1,96 @@ +# Vanilla voting NPCs + +Voting NPCs are public, named vanilla villagers that open the existing voting-site +selection. They do not receive votes, grant rewards, change totals, or replace +the normal `/vote` menus. No Citizens, fake-player internals, resource pack or +client mod is required. + +## Enable and create + +The feature is disabled by default. Add this to **Config.yml**, or change the +bundled default, then run `/av reload`: + +```yaml +VotingNPCs: + Enabled: true +``` + +Stand where the villager should appear and use: + +```text +/av npc create voting &aVote here +/av npc list +/av npc inspect voting +/av npc remove voting +``` + +IDs contain 1–40 lowercase letters, digits, underscores or hyphens and start with +a letter or digit. Names are optional, default to `&aVote`, and allow up to 128 +characters without control characters. `&` legacy colors are supported; names +are not interpreted as commands, PlaceholderAPI, JavaScript or rich-text actions. +Creation is player-only. List, inspect and remove also work from console. +Administration requires `VotingPlugin.Commands.AdminVote.NPC` (operator default) +or `VotingPlugin.Admin`. Normal vote-URL command permissions still apply when +a player interacts; an NPC never grants that permission. + +Right-click with the main hand to open the existing configured vote-URL GUI. +Already-cancelled interactions do not open it. Off-hand and repeated interaction +packets are suppressed. Trading, damage, combustion, targeting, vehicles and +teleportation and transformations (including lightning) are blocked only for the +exact tracked NPC entities. Ordinary +villagers, other plugins' NPCs and unrelated inventories are unaffected. + +## Persistence and lifecycle + +Definitions are saved asynchronously in **VotingNPCs.json** in the plugin data +directory. A success message confirms the definition write, not that a live +entity has already spawned. Inspect reports pending creation or cleanup as well +as created villagers. At most 64 definitions and 64 runtime reservations exist; +storage work also has a bounded queue. File ownership remains exclusive while +accepted writes drain during shutdown; shutdown does not wait for file I/O. + +Villagers themselves are **nonpersistent**. Definitions recreate them when their +world and surrounding chunks are available; the plugin never loads distant +chunks or scans worlds to find NPCs. There are no animation/tick loops. Chunk and +world unloads retire tracked entities; loading later reconstructs them. Changing +`Enabled` to false and reloading removes live NPCs while retaining definitions. +Reloading does not create a second entity for the same configured ID, including +when an earlier entity's cleanup is still pending. + +An invalid/unreadable definition file is unavailable, never an empty successful +load. Failed or uncertain writes stop further edits until a checked reload; +do not blindly repeat a command whose result was not confirmed. Preserve the +file, correct the problem and reload. Atomic file replacement and the existing +platform directory-sync policy are used. Abrupt shutdown has the filesystem's +normal durability limits; no universal exactly-once side effects are claimed. + +## Compatibility and limitations + +Spigot/Paper need the ordinary villager APIs, nonpersistent entities and the +`World.spawn(Location, Class, Consumer)` initialization API. Capability checks +reject unsupported implementations without changing normal voting. This does +not raise VotingPlugin's global Java or Minecraft requirement. It is a vanilla +villager, not a custom player skin or a Citizens-style scripted NPC. + +Folia creation uses the anchor region and verifies ownership of the villager's +bounds; cleanup follows the entity scheduler. A position crossing unavailable +or independently owned region bounds may remain unavailable. Choose a position +well inside a loaded chunk and retry through reload. No unsafe global-thread +entity removal is used. During final server shutdown, scheduler rejection can +prevent immediate removal; the nonpersistent entity is discarded at world +unload. External hot-disable/reload tools are not a supported substitute for a +server restart, particularly on Folia. Definition files remain intact. + +## Testing + +On a disposable server, enable the feature and create an NPC. Check main-hand +selection, ordinary vote permissions, cancellation by protections, off-hand +input and an unrelated villager. Inspect/list must agree with the saved ID. +Create/remove/recreate an ID while cleanup is delayed, reload both enabled and +disabled, unload/reload its chunk/world, then restart: one NPC should appear for +each enabled definition. Confirm removal affects only the selected NPC. + +Automated tests use real file storage, controlled Bukkit events, and explicit +region/entity scheduler queues. They are not visual Minecraft testing or proof +of an actual Paper/Folia deployment. Record native platform and client results +separately before asserting those scenarios were exercised. From d2d9256cd3a6f94394af1fb053a8cb1842e5b84b Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:01:02 -0600 Subject: [PATCH 2/6] Fix voting NPC path compatibility, permissions and protection --- .../votingplugin/npc/VotingNPCCommands.java | 28 ++++++-- .../votingplugin/npc/VotingNPCManager.java | 4 +- .../votingplugin/npc/VotingNPCRegistry.java | 17 +++-- .../votingplugin/npc/VotingNPCStore.java | 7 +- VotingPlugin/src/main/resources/plugin.yml | 4 ++ .../npc/VotingNPCCommandsTest.java | 69 +++++++++++++++++++ .../npc/VotingNPCManagerTest.java | 12 +++- .../npc/VotingNPCRegistryTest.java | 23 +++++++ .../votingplugin/npc/VotingNPCStoreTest.java | 17 +++++ docs/voting-npcs.md | 7 +- 10 files changed, 171 insertions(+), 17 deletions(-) diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java index e87d71e937..ce9e2813f0 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCCommands.java @@ -62,14 +62,14 @@ private static void executeOwned(VotingPluginMain plugin, CommandSender sender, String name = args.length > 3 ? String.join(" ", Arrays.copyOfRange(args, 3, args.length)) : "&aVote"; VotingNPCDefinition definition = new VotingNPCDefinition(args[2], UUID.randomUUID(), at.getWorld().getUID(), at.getX(), at.getY(), at.getZ(), at.getYaw(), at.getPitch(), name); - confirm(plugin, sender, manager.create(definition), + confirm(plugin, sender, manager, manager.create(definition), ignored -> "Saved voting NPC '" + definition.id() + "'. Its villager appears when the area is loaded and available."); } catch (IllegalArgumentException invalid) { sender.sendMessage("Use a lowercase ID of 1-40 letters, digits, underscores or hyphens, and a name of 1-128 characters without controls."); } break; case "remove": - confirm(plugin, sender, manager.remove(args[2]), removed -> removed + confirm(plugin, sender, manager, manager.remove(args[2]), removed -> removed ? "Removed saved voting NPC '" + args[2] + "'; exact entity cleanup has been requested." : "No voting NPC has that ID."); break; @@ -88,12 +88,32 @@ private static void executeOwned(VotingPluginMain plugin, CommandSender sender, } } - private static void confirm(VotingPluginMain plugin, CommandSender sender, CompletionStage completion, + private static String failureMessage(VotingNPCManager manager, Throwable failure) { + while ((failure instanceof java.util.concurrent.CompletionException + || failure instanceof java.util.concurrent.ExecutionException) && failure.getCause() != null) { + failure = failure.getCause(); + } + if (manager.available()) { + if (failure instanceof VotingNPCRegistry.ValidationFailure validation) { + return switch (validation.reason) { + case DUPLICATE_ID -> "That voting NPC ID already exists. Use a different ID or remove it first."; + case DUPLICATE_IDENTITY -> "That voting NPC identity already exists. No definition was changed."; + case LIMIT -> "The 64 voting NPC limit has been reached. Remove an NPC before creating another."; + }; + } + if (failure instanceof java.util.concurrent.RejectedExecutionException) { + return "Voting NPC storage is busy. Wait for pending changes to finish before retrying."; + } + } + return "Voting NPC change was not confirmed. Check storage and reload before retrying; no success is assumed."; + } + + private static void confirm(VotingPluginMain plugin, CommandSender sender, VotingNPCManager manager, CompletionStage completion, java.util.function.Function message) { completion.whenComplete((value, failure) -> onSender(plugin, sender, () -> { if (!allowed(sender)) return; sender.sendMessage(failure == null ? message.apply(value) - : "Voting NPC change was not confirmed. Check storage and reload before retrying; no success is assumed."); + : failureMessage(manager, failure)); })); } } diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java index 16f5fea8d7..74a705a02b 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java @@ -332,7 +332,7 @@ public void interact(PlayerInteractEntityEvent event) { @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) public void interactAt(PlayerInteractAtEntityEvent event) { interact(event); } - private boolean owned(Entity entity) { return entities.containsKey(entity.getUniqueId()); } + private boolean owned(Entity entity) { return entity != null && entities.containsKey(entity.getUniqueId()); } @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) public void damage(EntityDamageEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) @@ -342,7 +342,7 @@ public void interact(PlayerInteractEntityEvent event) { @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) public void transform(EntityTransformEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) - public void target(EntityTargetEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } + public void target(EntityTargetEvent event) { if (owned(event.getEntity()) || owned(event.getTarget())) event.setCancelled(true); } @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) public void blockInteract(EntityInteractEvent event) { if (owned(event.getEntity())) event.setCancelled(true); } @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false) diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java index e4297f59b7..f95855fcf5 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCRegistry.java @@ -24,6 +24,13 @@ * not that a world entity has spawned. Callbacks have no player/region ownership. */ final class VotingNPCRegistry implements AutoCloseable { + enum Rejection { DUPLICATE_ID, DUPLICATE_IDENTITY, LIMIT } + static final class ValidationFailure extends IllegalArgumentException { + private static final long serialVersionUID = 1L; + final Rejection reason; + ValidationFailure(Rejection reason) { super("Voting NPC request rejected"); this.reason = reason; } + } + private final Object admission = new Object(); private final ArrayBlockingQueue queue = new ArrayBlockingQueue<>(128); private final CountDownLatch stopped = new CountDownLatch(1); @@ -75,11 +82,11 @@ CompletionStage> reload() { CompletionStage create(VotingNPCDefinition definition) { return submit(true, () -> { if (definition == null) throw new IllegalArgumentException("Voting NPC definition is required"); - if (definitions.containsKey(definition.id())) throw new IllegalArgumentException("Voting NPC ID already exists"); + if (definitions.containsKey(definition.id())) throw new ValidationFailure(Rejection.DUPLICATE_ID); if (definitions.values().stream().anyMatch(value -> value.identity().equals(definition.identity()))) { - throw new IllegalArgumentException("Voting NPC identity already exists"); + throw new ValidationFailure(Rejection.DUPLICATE_IDENTITY); } - if (definitions.size() >= VotingNPCStore.MAX_NPCS) throw new RejectedExecutionException("Voting NPC definition limit reached"); + if (definitions.size() >= VotingNPCStore.MAX_NPCS) throw new ValidationFailure(Rejection.LIMIT); var next = new LinkedHashMap<>(definitions); next.put(definition.id(), definition); publish(next); @@ -149,8 +156,8 @@ private static CompletableFuture rejected(String message) { private void ownFile() throws IOException { if (lease != null && lease.isValid()) return; - for (Path parent = lockPath.getParent(); parent != null; parent = parent.getParent()) { - if (Files.isSymbolicLink(parent)) throw new IOException("Voting NPC owner path cannot cross a symbolic link"); + if (!Files.isDirectory(lockPath.getParent().toRealPath(), LinkOption.NOFOLLOW_LINKS)) { + throw new IOException("Voting NPC data directory is unavailable"); } FileChannel candidate = FileChannel.open(lockPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, LinkOption.NOFOLLOW_LINKS); diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java index 84c8658ee6..4119f848d0 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCStore.java @@ -102,10 +102,9 @@ synchronized void save(List definitions) throws IOException } private void checkParent() throws IOException { - for (Path parent = file.getParent(); parent != null; parent = parent.getParent()) { - if (Files.isSymbolicLink(parent)) throw new IOException("Voting NPC storage cannot cross a symbolic link"); - } - if (!Files.isDirectory(file.getParent(), LinkOption.NOFOLLOW_LINKS)) { + // Operators may place the data directory below a directory alias. Only + // the definitions and lease files themselves must remain non-links. + if (!Files.isDirectory(file.getParent().toRealPath(), LinkOption.NOFOLLOW_LINKS)) { throw new IOException("Voting NPC data directory is unavailable"); } } diff --git a/VotingPlugin/src/main/resources/plugin.yml b/VotingPlugin/src/main/resources/plugin.yml index 94f5dcc414..632fde76a5 100644 --- a/VotingPlugin/src/main/resources/plugin.yml +++ b/VotingPlugin/src/main/resources/plugin.yml @@ -170,6 +170,8 @@ permissions: description: Allows mod commands VotingPlugin.Admin: description: Allows admincommands + children: + VotingPlugin.Commands.AdminVote: true VotingPlugin.TopVoter.Ignore: description: Allows you to be exempt from top voter VotingPlugin.NoRemind: @@ -199,3 +201,5 @@ permissions: VotingPlugin.Commands.AdminVote.NPC: description: Create, inspect, list and remove saved vanilla voting NPCs default: op + children: + VotingPlugin.Commands.AdminVote: true diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java index 9fffa3f9a0..87431fe0d1 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java @@ -65,6 +65,39 @@ CommandHandler command(String action, int arguments) { for (var handler : f.handlers) assertEquals("VotingPlugin.Commands.AdminVote.NPC|VotingPlugin.Admin", handler.getPerm()); } + @Test void metadataGrantsTheAdminRootGateWithoutGrantingOtherAdminLeaves() throws Exception { + org.bukkit.plugin.PluginDescriptionFile description; + try (var input = getClass().getResourceAsStream("/plugin.yml")) { + assertNotNull(input); description = new org.bukkit.plugin.PluginDescriptionFile(input); + } + var permissions = description.getPermissions().stream().collect(java.util.stream.Collectors.toMap( + permission -> permission.getName().toLowerCase(java.util.Locale.ROOT), java.util.function.Function.identity())); + var plugin = mock(VotingPluginMain.class); when(plugin.isEnabled()).thenReturn(true); + var pm = mock(org.bukkit.plugin.PluginManager.class); + when(pm.getDefaultPermissions(false)).thenReturn(java.util.Set.of()); + when(pm.getPermission(anyString())).thenAnswer(call -> permissions.get(((String)call.getArgument(0)).toLowerCase(java.util.Locale.ROOT))); + try (var bukkit = mockStatic(org.bukkit.Bukkit.class)) { + bukkit.when(org.bukkit.Bukkit::getPluginManager).thenReturn(pm); + var server = mock(org.bukkit.Server.class); when(server.getPluginManager()).thenReturn(pm); + bukkit.when(org.bukkit.Bukkit::getServer).thenReturn(server); + for (String grant : List.of("VotingPlugin.Commands.AdminVote.NPC", "VotingPlugin.Admin")) { + var permissible = new org.bukkit.permissions.PermissibleBase(mock(org.bukkit.permissions.ServerOperator.class)); + permissible.addAttachment(plugin, grant, true); + CommandSender sender = mock(CommandSender.class); + when(sender.hasPermission(anyString())).thenAnswer(call -> permissible.hasPermission((String)call.getArgument(0))); + var root = new org.bukkit.command.Command("av") { + @Override public boolean execute(CommandSender sender, String label, String[] args) { return true; } + }; + root.setPermission("VotingPlugin.Commands.AdminVote"); + assertTrue(root.testPermissionSilent(sender), grant); + if (grant.endsWith(".NPC")) { + assertFalse(permissible.hasPermission("VotingPlugin.Commands.AdminVote.Edit.Config")); + assertFalse(permissible.hasPermission("VotingPlugin.Admin")); + } + } + } + } + @Test void capturesPlayerStateOnlyOnOwnerAndSuccessOnlyAfterDurableCompletion() { var f = new Fixture(); Player player = f.player(); var completion = new CompletableFuture(); @@ -103,6 +136,42 @@ CommandHandler command(String action, int arguments) { verify(player, never()).sendMessage(startsWith("Removed saved")); } + @Test void validationRejectionsHaveFixedActionableMessagesAndUnavailableStorageStaysConservative() { + for (var reason : VotingNPCRegistry.Rejection.values()) { + var f = new Fixture(); Player player = f.player(); + when(f.manager.create(any())).thenReturn(CompletableFuture.failedFuture( + new java.util.concurrent.CompletionException(new VotingNPCRegistry.ValidationFailure(reason)))); + f.command("Create", 3).execute(player, new String[] { "NPC", "Create", "voting" }); + f.tasks.remove().run(); f.tasks.remove().run(); + String expected = switch (reason) { + case DUPLICATE_ID -> "ID already exists"; + case DUPLICATE_IDENTITY -> "identity already exists"; + case LIMIT -> "64 voting NPC limit"; + }; + verify(player).sendMessage(contains(expected)); + verify(player, never()).sendMessage(contains("reload before retrying")); + } + var f = new Fixture(); Player player = f.player(); + var pending = new CompletableFuture(); + when(f.manager.create(any())).thenReturn(pending); + f.command("Create", 3).execute(player, new String[] { "NPC", "Create", "voting" }); f.tasks.remove().run(); + when(f.manager.available()).thenReturn(false); + pending.completeExceptionally(new java.util.concurrent.RejectedExecutionException("Synthetic")); f.tasks.remove().run(); + verify(player).sendMessage(contains("reload before retrying")); + verify(player, never()).sendMessage(contains("busy")); + } + + @Test void healthyQueueSaturationDoesNotTellTheAdministratorToReloadStorage() { + var f = new Fixture(); Player player = f.player(); + when(f.manager.create(any())).thenReturn(CompletableFuture.failedFuture( + new java.util.concurrent.RejectedExecutionException("Synthetic queue rejection"))); + f.command("Create", 3).execute(player, new String[] { "NPC", "Create", "voting" }); + f.tasks.remove().run(); f.tasks.remove().run(); + verify(player).sendMessage(contains("busy")); + verify(player, never()).sendMessage(contains("reload before retrying")); + verify(player, never()).sendMessage(contains("Synthetic")); + } + @Test void invalidDefinitionAndConsoleCreationDoNotMutate() { var f = new Fixture(); Player player = f.player(); f.command("Create", 3).execute(player, new String[] { "NPC", "Create", "bad id" }); f.tasks.remove().run(); diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java index 19d6fad11f..75a1bd9411 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java @@ -91,7 +91,7 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception verify(spawned.getFirst()).setPersistent(false); verify(spawned.getFirst()).setAI(false); verify(spawned.getFirst()).setCustomName("\u00a7aVote"); - Player player = mock(Player.class); + Player player = mock(Player.class); when(player.getUniqueId()).thenReturn(new UUID(0, 100)); Villager unrelated = mock(Villager.class); when(unrelated.getUniqueId()).thenReturn(new UUID(0, 99)); var interact = new PlayerInteractEntityEvent(player, unrelated, EquipmentSlot.HAND); manager.interact(interact); assertFalse(interact.isCancelled()); @@ -99,6 +99,16 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception manager.damage(damage); verify(damage, never()).setCancelled(anyBoolean()); var protectedDamage = mock(EntityDamageEvent.class); when(protectedDamage.getEntity()).thenReturn(spawned.getFirst()); manager.damage(protectedDamage); verify(protectedDamage).setCancelled(true); + var hostileTarget = new org.bukkit.event.entity.EntityTargetEvent(unrelated, spawned.getFirst(), + org.bukkit.event.entity.EntityTargetEvent.TargetReason.CLOSEST_ENTITY); + manager.target(hostileTarget); + assertTrue(hostileTarget.isCancelled(), "ordinary mobs must not target a tracked NPC"); + var normalTarget = new org.bukkit.event.entity.EntityTargetEvent(unrelated, player, + org.bukkit.event.entity.EntityTargetEvent.TargetReason.CLOSEST_ENTITY); + manager.target(normalTarget); assertFalse(normalTarget.isCancelled()); + var clearedTarget = new org.bukkit.event.entity.EntityTargetEvent(unrelated, null, + org.bukkit.event.entity.EntityTargetEvent.TargetReason.FORGOT_TARGET); + manager.target(clearedTarget); assertFalse(clearedTarget.isCancelled()); var witch = mock(org.bukkit.entity.Witch.class); var transform = new org.bukkit.event.entity.EntityTransformEvent(spawned.getFirst(), List.of(witch), org.bukkit.event.entity.EntityTransformEvent.TransformReason.LIGHTNING); diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java index 28bee20064..5112206d93 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCRegistryTest.java @@ -43,6 +43,29 @@ private static void waitOn(CountDownLatch latch) throws IOException { } } + @Test void linkedDirectoryAliasesShareTheSameExclusiveOwnerAndLeafLockLinksAreRejected() throws Exception { + Path real = Files.createDirectory(directory.resolve("real")); + Path linked = Files.createSymbolicLink(directory.resolve("linked"), real); + Path file = linked.resolve("npcs.json"); + var first = new VotingNPCRegistry(file); + try { + await(first.initialLoad()); await(first.create(definition(1))); + var alias = new VotingNPCRegistry(real.resolve("npcs.json")); + try { assertThrows(ExecutionException.class, () -> await(alias.initialLoad())); } + finally { close(alias); } + } finally { close(first); } + var recovered = new VotingNPCRegistry(file); + try { assertEquals(List.of(definition(1)), await(recovered.initialLoad())); } + finally { close(recovered); } + Path lock = linked.resolve("other.json.lock"); + Path unrelated = real.resolve("unrelated"); Files.writeString(unrelated, "unchanged"); + Files.createSymbolicLink(lock, unrelated); + var rejected = new VotingNPCRegistry(linked.resolve("other.json")); + try { assertThrows(ExecutionException.class, () -> await(rejected.initialLoad())); } + finally { close(rejected); } + assertEquals("unchanged", Files.readString(unrelated)); + } + @Test void orderedDefinitionsAreDurableAndDuplicateRequestsDoNotOverwrite() throws Exception { Path file = directory.resolve("npcs.json"); var registry = new VotingNPCRegistry(file); diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java index 9f37206a10..51c464997e 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCStoreTest.java @@ -130,6 +130,23 @@ private VotingNPCDefinition definition(int id) { assertEquals("unrelated", Files.readString(target)); } + @Test void linkedDirectoriesAllowDurableDefinitionsButLeafLinksStayRejected() throws Exception { + Path real = Files.createDirectory(directory.resolve("real")); + Path linked = Files.createSymbolicLink(directory.resolve("linked"), real); + Path nested = Files.createDirectory(real.resolve("data")); + for (Path parent : List.of(linked, linked.resolve("data"))) { + Path file = parent.resolve("npcs.json"); + var store = new VotingNPCStore(file); + assertTrue(store.load().isEmpty()); + store.save(List.of(definition(1))); + assertEquals(List.of(definition(1)), new VotingNPCStore(file).load()); + } + Path leaf = linked.resolve("alias.json"); + Files.createSymbolicLink(leaf, nested.resolve("npcs.json")); + assertThrows(IOException.class, () -> new VotingNPCStore(leaf).load()); + assertEquals(List.of(definition(1)), new VotingNPCStore(nested.resolve("npcs.json")).load()); + } + @Test void invalidDefinitionsAreRejectedBeforePublication() { assertThrows(IllegalArgumentException.class, () -> new VotingNPCDefinition("bad id", new UUID(0, 1), new UUID(1, 0), 0, 64, 0, 0, 0, "Vote")); diff --git a/docs/voting-npcs.md b/docs/voting-npcs.md index 842ef785f9..9179d021cc 100644 --- a/docs/voting-npcs.md +++ b/docs/voting-npcs.md @@ -30,7 +30,8 @@ characters without control characters. `&` legacy colors are supported; names are not interpreted as commands, PlaceholderAPI, JavaScript or rich-text actions. Creation is player-only. List, inspect and remove also work from console. Administration requires `VotingPlugin.Commands.AdminVote.NPC` (operator default) -or `VotingPlugin.Admin`. Normal vote-URL command permissions still apply when +or `VotingPlugin.Admin`. Both also permit entering the `/av` command root; the +NPC permission does not grant other administrative subcommands. Normal vote-URL command permissions still apply when a player interacts; an NPC never grants that permission. Right-click with the main hand to open the existing configured vote-URL GUI. @@ -57,6 +58,10 @@ world unloads retire tracked entities; loading later reconstructs them. Changing Reloading does not create a second entity for the same configured ID, including when an earlier entity's cleanup is still pending. +Operator-configured directory aliases are supported; the definitions and lock +files themselves must not be symbolic links. Duplicate IDs, capacity limits and +a busy worker report fixed actionable messages without requiring a reload. + An invalid/unreadable definition file is unavailable, never an empty successful load. Failed or uncertain writes stop further edits until a checked reload; do not blindly repeat a command whose result was not confirmed. Preserve the From 1d89eeca4d7a42d3dfa3f3752548522054e77f13 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:16:53 -0600 Subject: [PATCH 3/6] Retire voting NPCs across their occupied chunk bounds --- .../votingplugin/npc/VotingNPCManager.java | 28 +++++--- .../npc/VotingNPCCommandsTest.java | 2 + .../npc/VotingNPCManagerTest.java | 69 +++++++++++++++++++ docs/voting-npcs.md | 5 +- 4 files changed, 93 insertions(+), 11 deletions(-) diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java index 74a705a02b..a8946c7088 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java @@ -189,7 +189,7 @@ private void reconcile() { List snapshot = definitions(); Set chunks = new HashSet<>(); Set worlds = new HashSet<>(); - for (var value : snapshot) { chunks.add(key(value)); worlds.add(value.world()); } + for (var value : snapshot) { chunks.addAll(occupiedChunks(value)); worlds.add(value.world()); } unloadingChunks.retainAll(chunks); unloadingWorlds.retainAll(worlds); for (RuntimeNPC npc : resources.values()) if (!current(npc)) retire(npc); @@ -200,10 +200,21 @@ private void reconcile() { }, this::report); } - private static ChunkKey key(VotingNPCDefinition value) { return new ChunkKey(value.world(), value.chunkX(), value.chunkZ()); } + /** Same conservative half-block bounds used for creation ownership and chunk availability. */ + private static Set occupiedChunks(VotingNPCDefinition value) { + Set chunks = new HashSet<>(); + for (double dx : new double[] { -.5, .5 }) for (double dz : new double[] { -.5, .5 }) { + chunks.add(new ChunkKey(value.world(), (int) Math.floor(value.x() + dx) >> 4, + (int) Math.floor(value.z() + dz) >> 4)); + } + return chunks; + } + private boolean unloading(VotingNPCDefinition value) { + return occupiedChunks(value).stream().anyMatch(unloadingChunks::contains); + } private void ensure(VotingNPCDefinition value) { - if (!available() || !definitions().contains(value) || unloadingWorlds.contains(value.world()) || unloadingChunks.contains(key(value))) return; + if (!available() || !definitions().contains(value) || unloadingWorlds.contains(value.world()) || unloading(value)) return; World world = Bukkit.getWorld(value.world()); if (world == null) return; RuntimeNPC npc = new RuntimeNPC(value, world, configuration); @@ -225,7 +236,7 @@ private void ensure(VotingNPCDefinition value) { private boolean loadedArea(RuntimeNPC npc) { World world = npc.anchor.getWorld(); if (Bukkit.getWorld(npc.definition.world()) != world || unloadingWorlds.contains(npc.definition.world()) - || unloadingChunks.contains(key(npc.definition))) return false; + || unloading(npc.definition)) return false; for (double dx : new double[] { -.5, .5 }) for (double dz : new double[] { -.5, .5 }) { Location edge = npc.anchor.clone().add(dx, 0, dz); if (!scheduler.owns(edge) || !world.isChunkLoaded(edge.getBlockX() >> 4, edge.getBlockZ() >> 4)) return false; @@ -353,17 +364,16 @@ public void interact(PlayerInteractEntityEvent event) { public void chunkLoad(ChunkLoadEvent event) { UUID world = event.getWorld().getUID(); unloadingChunks.remove(new ChunkKey(world, event.getChunk().getX(), event.getChunk().getZ())); - for (var value : definitions()) if (value.world().equals(world) - && Math.abs(value.chunkX() - event.getChunk().getX()) <= 1 - && Math.abs(value.chunkZ() - event.getChunk().getZ()) <= 1) ensure(value); + ChunkKey chunk = new ChunkKey(world, event.getChunk().getX(), event.getChunk().getZ()); + for (var value : definitions()) if (occupiedChunks(value).contains(chunk)) ensure(value); } @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) public void chunkUnload(ChunkUnloadEvent event) { ChunkKey chunk = new ChunkKey(event.getWorld().getUID(), event.getChunk().getX(), event.getChunk().getZ()); - if (definitions().stream().noneMatch(value -> key(value).equals(chunk))) return; + if (definitions().stream().noneMatch(value -> occupiedChunks(value).contains(chunk))) return; unloadingChunks.add(chunk); - for (RuntimeNPC npc : resources.values()) if (key(npc.definition).equals(chunk)) retire(npc); + for (RuntimeNPC npc : resources.values()) if (occupiedChunks(npc.definition).contains(chunk)) retire(npc); } @EventHandler(priority = EventPriority.MONITOR) diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java index 87431fe0d1..fe1f8d98e2 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCCommandsTest.java @@ -48,6 +48,8 @@ Player player() { when(player.isOnline()).thenReturn(true); when(player.hasPermission("VotingPlugin.Commands.AdminVote.NPC")).thenReturn(true); World world = mock(World.class); when(world.getUID()).thenReturn(new UUID(1, 0)); + // Location holds its world weakly; retain it as a real online player does. + when(player.getWorld()).thenReturn(world); when(player.getLocation()).thenReturn(new Location(world, 4, 64, 4)); return player; } diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java index 75a1bd9411..0ffcc2a777 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java @@ -220,6 +220,75 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception } } + @Test void occupiedNeighbourUnloadRetiresAndReloadReconstructsOnceAtBothCoordinateSigns() throws Exception { + for (double position : new double[] {15.75, -.25}) { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("boundary-" + (position > 0) + ".json")); + await(registry.initialLoad()); + var definition = new VotingNPCDefinition("voting", new UUID(0, 1), WORLD, position, 64, position, 0, 0, "Vote"); + await(registry.create(definition)); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + manager.chunkUnload(new ChunkUnloadEvent(atChunk(world, 7, 7))); + assertTrue(scheduler.entities.isEmpty(), "unrelated chunk unload must leave NPC alone"); + int neighbour = definition.chunkX() + 1; + manager.chunkUnload(new ChunkUnloadEvent(atChunk(world, neighbour, neighbour))); + assertEquals(1, scheduler.entities.size(), "occupied adjacent chunk must trigger entity-owned cleanup"); + // Bukkit may still report the unloading chunk as loaded during its event. + manager.chunkLoad(new ChunkLoadEvent(atChunk(world, definition.chunkX(), definition.chunkZ()), false)); + scheduler.runNext(scheduler.entities, "entity"); + assertEquals(0, manager.runtimeCount()); assertTrue(scheduler.regions.isEmpty()); + manager.chunkLoad(new ChunkLoadEvent(atChunk(world, neighbour, neighbour), false)); + manager.chunkLoad(new ChunkLoadEvent(atChunk(world, neighbour, neighbour), false)); + scheduler.runNext(scheduler.regions, "region"); + assertEquals(2, spawned.size()); assertEquals(1, manager.runtimeCount()); + verify(world, never()).getChunkAt(anyInt(), anyInt()); verify(world, never()).loadChunk(anyInt(), anyInt()); + } finally { + manager.close(); if (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + } + + @Test void neighbourUnloadFenceSurvivesReconciliationAndBlocksDeferredCreation() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("pending-boundary.json")); await(registry.initialLoad()); + await(registry.create(new VotingNPCDefinition("voting", new UUID(0, 1), WORLD, 15.75, 64, 15.75, 0, 0, "Vote"))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + manager.chunkUnload(new ChunkUnloadEvent(atChunk(world, 1, 1))); + scheduler.runNext(scheduler.regions, "region"); + assertTrue(spawned.isEmpty(), "a deferred creation cannot cross an occupied chunk unload"); + assertEquals(0, manager.runtimeCount()); + await(manager.create(new VotingNPCDefinition("other", new UUID(0, 2), WORLD, 40, 64, 40, 0, 0, "Vote"))); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + assertEquals(1, spawned.size(), "unrelated mutation must not erase the first NPC's occupied-chunk fence"); + assertTrue(scheduler.regions.isEmpty()); + } finally { + manager.close(); while (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + + private static Chunk atChunk(World world, int x, int z) { + Chunk chunk = mock(Chunk.class); when(chunk.getWorld()).thenReturn(world); + when(chunk.getX()).thenReturn(x); when(chunk.getZ()).thenReturn(z); return chunk; + } + @Test void disabledFeatureDoesNotReadStorageOrRegisterAnyRuntimeWork() { var plugin = mock(VotingPluginMain.class); var scheduler = new Scheduler(plugin); diff --git a/docs/voting-npcs.md b/docs/voting-npcs.md index 9179d021cc..7d58de47f4 100644 --- a/docs/voting-npcs.md +++ b/docs/voting-npcs.md @@ -52,8 +52,9 @@ accepted writes drain during shutdown; shutdown does not wait for file I/O. Villagers themselves are **nonpersistent**. Definitions recreate them when their world and surrounding chunks are available; the plugin never loads distant -chunks or scans worlds to find NPCs. There are no animation/tick loops. Chunk and -world unloads retire tracked entities; loading later reconstructs them. Changing +chunks or scans worlds to find NPCs. There are no animation/tick loops. Unloading +any chunk occupied by the checked half-block bounds retires its tracked entity; +chunk/world loading later reconstructs it. Changing `Enabled` to false and reloading removes live NPCs while retaining definitions. Reloading does not create a second entity for the same configured ID, including when an earlier entity's cleanup is still pending. From 8bb8851340b56219c4a3cb8f4d33a381c0afb840 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:49:35 -0600 Subject: [PATCH 4/6] Retry deferred voting NPC cleanup after lifecycle recovery --- .../votingplugin/npc/VotingNPCManager.java | 16 +++++++- .../npc/VotingNPCManagerTest.java | 40 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java index a8946c7088..c69b144352 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java @@ -186,6 +186,7 @@ private boolean current(RuntimeNPC npc) { } private void reconcile() { + retryPendingCleanup(); List snapshot = definitions(); Set chunks = new HashSet<>(); Set worlds = new HashSet<>(); @@ -287,12 +288,24 @@ private void retire(RuntimeNPC npc) { } private void cleanupFailed(RuntimeNPC npc, Throwable failure) { - synchronized (npc) { npc.cleanupQueued = false; } + synchronized (npc) { + npc.cleanupQueued = false; + npc.reconstructRequested = true; + } // Keep the exact identity and capacity reservation. Rejected scheduling // is not successful removal and must not permit duplicate reconstruction. report(failure); } + private void retryPendingCleanup() { + for (RuntimeNPC npc : resources.values()) { + synchronized (npc) { + if (!npc.closed || !npc.reconstructRequested || npc.cleanupQueued || npc.entity == null) continue; + } + retire(npc); + } + } + private void forget(RuntimeNPC npc) { boolean reconstruct; synchronized (npc) { @@ -364,6 +377,7 @@ public void interact(PlayerInteractEntityEvent event) { public void chunkLoad(ChunkLoadEvent event) { UUID world = event.getWorld().getUID(); unloadingChunks.remove(new ChunkKey(world, event.getChunk().getX(), event.getChunk().getZ())); + retryPendingCleanup(); ChunkKey chunk = new ChunkKey(world, event.getChunk().getX(), event.getChunk().getZ()); for (var value : definitions()) if (occupiedChunks(value).contains(chunk)) ensure(value); } diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java index 0ffcc2a777..fc12a6374e 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java @@ -150,8 +150,13 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.global, "global"); assertEquals(1, spawned.size()); assertEquals(1, manager.runtimeCount()); verify(spawned.getFirst(), never()).remove(); + // A rejected cleanup keeps ownership; repeated load notifications may + // request a retry, but must never create a successor or duplicate task. + manager.chunkLoad(new ChunkLoadEvent(chunk(world), false)); + manager.chunkLoad(new ChunkLoadEvent(chunk(world), false)); + assertEquals(1, spawned.size()); assertEquals(1, manager.runtimeCount()); scheduler.rejectCleanup = false; - manager.configure(true); scheduler.runNext(scheduler.global, "global"); + manager.chunkLoad(new ChunkLoadEvent(chunk(world), false)); scheduler.runNext(scheduler.entities, "entity"); verify(spawned.getFirst()).remove(); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); @@ -164,6 +169,39 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception } } + @Test void transientUnloadCleanupRetriesAtLoadAndCoalescesUntilRemovalFinishes() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("npcs.json")); await(registry.initialLoad()); + await(registry.create(definition(1))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + Chunk chunk = chunk(world); scheduler.rejectCleanup = true; + manager.chunkUnload(new ChunkUnloadEvent(chunk)); + assertEquals(1, manager.runtimeCount()); assertTrue(scheduler.global.isEmpty()); + assertTrue(scheduler.entities.isEmpty(), "rejection must not create an autonomous retry loop"); + scheduler.rejectCleanup = false; + manager.chunkLoad(new ChunkLoadEvent(chunk, false)); + manager.chunkLoad(new ChunkLoadEvent(chunk, false)); + assertEquals(1, scheduler.entities.size(), "a queued exact predecessor cleanup must coalesce"); + assertEquals(1, spawned.size()); + scheduler.runNext(scheduler.entities, "entity"); verify(spawned.getFirst()).remove(); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + assertEquals(2, spawned.size()); assertEquals(1, manager.runtimeCount()); + assertTrue(scheduler.entities.isEmpty()); assertTrue(scheduler.global.isEmpty()); + } finally { + scheduler.rejectCleanup = false; manager.close(); + if (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + @Test void unloadedChunksNeverSpawnOrLoadAndLaterLoadReconstructsOnce() throws Exception { var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); try (var bukkit = mockStatic(Bukkit.class)) { From 70106256018deba2061823578354b6138c19384a Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:15:56 -0600 Subject: [PATCH 5/6] Wake pending voting NPCs when runtime capacity is released --- .../votingplugin/npc/VotingNPCManager.java | 36 +++++++-- .../npc/VotingNPCManagerTest.java | 77 +++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java index c69b144352..cbdaf65ad2 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java @@ -49,6 +49,8 @@ public final class VotingNPCManager implements Listener, AutoCloseable { private final VotingNPCScheduler scheduler; private final Consumer open; private final Map resources = new ConcurrentHashMap<>(); + private final Set capacityDemand = ConcurrentHashMap.newKeySet(); + private final AtomicBoolean capacityWakeQueued = new AtomicBoolean(); private final Map entities = new ConcurrentHashMap<>(); private final Set unloadingChunks = ConcurrentHashMap.newKeySet(); private final Set unloadingWorlds = ConcurrentHashMap.newKeySet(); @@ -187,6 +189,7 @@ private boolean current(RuntimeNPC npc) { private void reconcile() { retryPendingCleanup(); + capacityDemand.retainAll(definitions().stream().map(VotingNPCDefinition::id).toList()); List snapshot = definitions(); Set chunks = new HashSet<>(); Set worlds = new HashSet<>(); @@ -223,8 +226,12 @@ private void ensure(VotingNPCDefinition value) { synchronized (resources) { previous = resources.get(value.id()); if (previous == null) { - if (resources.size() >= VotingNPCStore.MAX_NPCS) return; + if (resources.size() >= VotingNPCStore.MAX_NPCS) { + if (capacityDemand.size() < VotingNPCStore.MAX_NPCS) capacityDemand.add(value.id()); + return; + } resources.put(value.id(), npc); + capacityDemand.remove(value.id()); } } if (previous != null) { @@ -248,7 +255,11 @@ private boolean loadedArea(RuntimeNPC npc) { private void spawn(RuntimeNPC npc) { try { synchronized (npc) { - if (npc.closed || !current(npc) || !loadedArea(npc)) { forget(npc); return; } + if (npc.closed || !current(npc) || !loadedArea(npc)) { + boolean capacityReleased = !npc.closed && !current(npc); + forget(npc, capacityReleased); + return; + } if (npc.entity != null) return; Villager spawned = npc.anchor.getWorld().spawn(npc.anchor, Villager.class, villager -> { npc.entity = villager; @@ -277,7 +288,7 @@ private void retire(RuntimeNPC npc) { synchronized (npc) { npc.closed = true; entity = npc.entity; - if (entity == null) { forget(npc); return; } + if (entity == null) { forget(npc, false); return; } if (npc.cleanupQueued) return; npc.cleanupQueued = true; } @@ -306,20 +317,33 @@ private void retryPendingCleanup() { } } - private void forget(RuntimeNPC npc) { - boolean reconstruct; + private void forget(RuntimeNPC npc) { forget(npc, false); } + + private void forget(RuntimeNPC npc, boolean capacityReleased) { + boolean reconstruct, hadEntity, removed; synchronized (npc) { reconstruct = npc.entity != null && (npc.reconstructRequested || npc.configuration != configuration || definitions().stream().anyMatch(value -> value.id().equals(npc.definition.id()) && !value.equals(npc.definition))); + hadEntity = npc.entity != null; if (npc.entityId != null) entities.remove(npc.entityId, npc); npc.entity = null; - resources.remove(npc.definition.id(), npc); + removed = resources.remove(npc.definition.id(), npc); } // A successor sharing this configured ID must wait for exact predecessor // cleanup. Missing/unloaded spawn attempts do not recursively retry. if (reconstruct && available()) scheduler.global(() -> { for (var value : definitions()) if (value.id().equals(npc.definition.id())) ensure(value); }, this::report); + if (removed && (hadEntity || capacityReleased) && !capacityDemand.isEmpty() && available() + && capacityWakeQueued.compareAndSet(false, true)) { + scheduler.global(() -> { + capacityWakeQueued.set(false); + for (var value : definitions()) if (capacityDemand.contains(value.id())) ensure(value); + }, failure -> { + capacityWakeQueued.set(false); + report(failure); + }); + } } private void report(Throwable failure) { diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java index fc12a6374e..fd8f78e682 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java @@ -43,6 +43,10 @@ private static VotingNPCDefinition definition(int identity) { return new VotingNPCDefinition("voting", new UUID(0, identity), WORLD, 8, 64, 8, 0, 0, "&aVote"); } + private static VotingNPCDefinition definitionAt(int identity, double x) { + return new VotingNPCDefinition("npc-" + identity, new UUID(0, identity), WORLD, x, 64, 8, 0, 0, "&aVote"); + } + private static T await(CompletionStage stage) throws Exception { return stage.toCompletableFuture().get(5, TimeUnit.SECONDS); } @@ -131,6 +135,33 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception } } + @Test void freedFullCapacityWakesOnePendingDifferentId() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("capacity.json")); await(registry.initialLoad()); + for (int i = 1; i <= VotingNPCStore.MAX_NPCS; i++) await(registry.create(definitionAt(i, i * 2))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + for (int i = 0; i < VotingNPCStore.MAX_NPCS; i++) scheduler.runNext(scheduler.regions, "region"); + assertEquals(VotingNPCStore.MAX_NPCS, manager.runtimeCount()); + await(manager.remove("npc-1")); await(manager.create(definitionAt(1000, 2000))); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.global, "global"); + scheduler.runNext(scheduler.entities, "entity"); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.regions, "region"); + assertEquals(VotingNPCStore.MAX_NPCS, manager.runtimeCount()); + assertEquals(VotingNPCStore.MAX_NPCS + 1, spawned.size()); + } finally { + manager.close(); while (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + @Test void successorWaitsForExactPredecessorCleanupAndCleanupRejectionRetainsOwnership() throws Exception { var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); try (var bukkit = mockStatic(Bukkit.class)) { @@ -202,6 +233,52 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception } } + @Test void queuedRemovedReservationWakesPendingCapacityWithoutEntity() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("queued-capacity.json")); await(registry.initialLoad()); + for (int i = 1; i <= VotingNPCStore.MAX_NPCS; i++) await(registry.create(definitionAt(i, i * 2))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + await(manager.remove("npc-1")); await(manager.create(definitionAt(1000, 2000))); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.global, "global"); + for (int i = 0; i <= VotingNPCStore.MAX_NPCS; i++) scheduler.runNext(scheduler.regions, "region"); + assertTrue(scheduler.regions.isEmpty()); assertTrue(scheduler.global.isEmpty()); + assertEquals(VotingNPCStore.MAX_NPCS, manager.runtimeCount()); + assertEquals(VotingNPCStore.MAX_NPCS, spawned.size()); + } finally { + manager.close(); while (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + + @Test void unloadedQueuedReservationWithPendingCapacityDoesNotRetry() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(false); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("unloaded-capacity.json")); await(registry.initialLoad()); + for (int i = 1; i <= VotingNPCStore.MAX_NPCS; i++) await(registry.create(definitionAt(i, i * 2))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + await(manager.remove("npc-1")); await(manager.create(definitionAt(1000, 2000))); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.global, "global"); + for (int i = 0; i <= VotingNPCStore.MAX_NPCS; i++) scheduler.runNext(scheduler.regions, "region"); + assertEquals(0, spawned.size()); assertTrue(scheduler.global.isEmpty()); + assertTrue(scheduler.regions.isEmpty()); assertEquals(0, manager.runtimeCount()); + } finally { manager.close(); assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); } + } + } + @Test void unloadedChunksNeverSpawnOrLoadAndLaterLoadReconstructsOnce() throws Exception { var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); try (var bukkit = mockStatic(Bukkit.class)) { From 2f6cb104bf8d5cc15ee39dd812099b8be9dc1e73 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:38:48 -0600 Subject: [PATCH 6/6] Wake waiting voting NPCs after unloaded reservations --- .../votingplugin/npc/VotingNPCManager.java | 5 +-- .../npc/VotingNPCManagerTest.java | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java index cbdaf65ad2..6960dabcb3 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/npc/VotingNPCManager.java @@ -320,11 +320,10 @@ private void retryPendingCleanup() { private void forget(RuntimeNPC npc) { forget(npc, false); } private void forget(RuntimeNPC npc, boolean capacityReleased) { - boolean reconstruct, hadEntity, removed; + boolean reconstruct, removed; synchronized (npc) { reconstruct = npc.entity != null && (npc.reconstructRequested || npc.configuration != configuration || definitions().stream().anyMatch(value -> value.id().equals(npc.definition.id()) && !value.equals(npc.definition))); - hadEntity = npc.entity != null; if (npc.entityId != null) entities.remove(npc.entityId, npc); npc.entity = null; removed = resources.remove(npc.definition.id(), npc); @@ -334,7 +333,7 @@ private void forget(RuntimeNPC npc, boolean capacityReleased) { if (reconstruct && available()) scheduler.global(() -> { for (var value : definitions()) if (value.id().equals(npc.definition.id())) ensure(value); }, this::report); - if (removed && (hadEntity || capacityReleased) && !capacityDemand.isEmpty() && available() + if (removed && !capacityDemand.isEmpty() && available() && capacityWakeQueued.compareAndSet(false, true)) { scheduler.global(() -> { capacityWakeQueued.set(false); diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java index fd8f78e682..e15325b029 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/npc/VotingNPCManagerTest.java @@ -162,6 +162,44 @@ void runNext(LinkedBlockingQueue queue, String phase) throws Exception } } + @Test void unloadedCurrentReservationWakesDifferentPendingIdWithoutRetryingItself() throws Exception { + var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + try (var bukkit = mockStatic(Bukkit.class)) { + World world = mock(World.class); when(world.getUID()).thenReturn(WORLD); + bukkit.when(() -> Bukkit.getWorld(WORLD)).thenReturn(world); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + var scheduler = new Scheduler(plugin); List spawned = installSpawner(world, scheduler); + var registry = new VotingNPCRegistry(directory.resolve("unloaded-current-capacity.json")); await(registry.initialLoad()); + for (int i = 1; i < VotingNPCStore.MAX_NPCS; i++) await(registry.create(definitionAt(i, i * 2))); + var manager = new VotingNPCManager(plugin, scheduler, registry, ignored -> { }); + try { + manager.configure(true); scheduler.runNext(scheduler.global, "global"); + for (int i = 1; i < VotingNPCStore.MAX_NPCS; i++) scheduler.runNext(scheduler.regions, "region"); + await(manager.create(definitionAt(64, 128))); + scheduler.runNext(scheduler.global, "global"); + assertEquals(64, manager.runtimeCount()); assertEquals(1, scheduler.regions.size()); + await(manager.remove("npc-1")); await(manager.create(definitionAt(1000, 2000))); + scheduler.runNext(scheduler.global, "global"); scheduler.runNext(scheduler.global, "global"); + assertEquals(1, scheduler.entities.size()); assertTrue(scheduler.global.isEmpty()); + assertEquals(1, scheduler.regions.size()); + // The existing queued reservation is now unloaded; the new target remains loaded. + when(world.isChunkLoaded(anyInt(), anyInt())).thenAnswer(call -> (Integer) call.getArgument(0) != 8); + scheduler.runNext(scheduler.regions, "region"); + assertEquals(63, manager.runtimeCount()); + assertEquals(1, scheduler.global.size(), "Released capacity must wake the different waiting ID"); + scheduler.runNext(scheduler.global, "global"); + assertEquals(1, scheduler.regions.size(), "Only the pending ID may retry, not the unloaded reservation"); + scheduler.runNext(scheduler.regions, "region"); + assertEquals(64, spawned.size()); assertEquals(64, manager.runtimeCount()); + assertTrue(scheduler.global.isEmpty()); assertTrue(scheduler.regions.isEmpty()); + verify(world).spawn(argThat(anchor -> anchor.getX() == 2000), eq(Villager.class), any(Consumer.class)); + } finally { + manager.close(); while (!scheduler.entities.isEmpty()) scheduler.runNext(scheduler.entities, "entity"); + assertTrue(registry.awaitClosed(5, TimeUnit.SECONDS)); + } + } + } + @Test void successorWaitsForExactPredecessorCleanupAndCleanupRejectionRetainsOwnership() throws Exception { var plugin = mock(VotingPluginMain.class); when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); try (var bukkit = mockStatic(Bukkit.class)) {