refactor: streamline data lock acquisition and cleanup logic in SpawnerLootGenerator and SpawnerRangeChecker

Author: ptthanh02

core/src/main/java/github/nighter/smartspawner/spawner/lootgen/SpawnerLootGenerator.java

@@ -41,26 +41,21 @@ public void spawnLootToSpawner(SpawnerData spawner) {
         try {
             // Acquire dataLock to safely read spawn timing and configuration values
             // Use tryLock with short timeout to avoid blocking
-            boolean dataLockAcquired = false;
             try {
-                dataLockAcquired = spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS);
+                if (!spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS)) {
+                    // dataLock is held (likely stack size change), skip this cycle
+                    return;
+                }
             } catch (InterruptedException e) {
                 Thread.currentThread().interrupt();
                 return;
             }
-            
-            if (!dataLockAcquired) {
-                // dataLock is held (likely stack size change), skip this cycle
-                return;
-            }
-            
+
             // Declare variables outside the try block so they're accessible in the async lambda
             final long currentTime = System.currentTimeMillis();
             final long spawnTime;
-            final EntityType entityType;
             final int minMobs;
             final int maxMobs;
-            final String spawnerId;
             final AtomicInteger usedSlots;
             final AtomicInteger maxSlots;
 
@@ -81,10 +76,8 @@ public void spawnLootToSpawner(SpawnerData spawner) {
                 }
 
                 // Important: Store the current values we need for async processing
-                entityType = spawner.getEntityType();
                 minMobs = spawner.getMinMobs();
                 maxMobs = spawner.getMaxMobs();
-                spawnerId = spawner.getSpawnerId();
                 // Store currentTime to update lastSpawnTime after successful loot addition
                 spawnTime = currentTime;
             } finally {
@@ -289,7 +282,7 @@ private List<ItemStack> limitItemsToAvailableSlots(List<ItemStack> items, Spawne
                 int remainingSlots = maxSlots - calculateSlots(simulatedInventory);
                 if (remainingSlots > 0) {
                     // Maximum items we can add in the remaining slots
-                    long maxAddAmount = remainingSlots * maxStackSize - (currentAmount % maxStackSize);
+                    long maxAddAmount = (long) remainingSlots * maxStackSize - (currentAmount % maxStackSize);
                     if (maxAddAmount > 0) {
                         // Create a partial item
                         ItemStack partialItem = item.clone();
@@ -348,11 +341,9 @@ private void handleGuiUpdates(SpawnerData spawner) {
             Location loc = spawner.getSpawnerLocation();
             World world = loc.getWorld();
             if (world != null) {
-                Scheduler.runLocationTask(loc, () -> {
-                    world.spawnParticle(Particle.HAPPY_VILLAGER,
-                            loc.clone().add(0.5, 0.5, 0.5),
-                            10, 0.3, 0.3, 0.3, 0);
-                });
+                Scheduler.runLocationTask(loc, () -> world.spawnParticle(Particle.HAPPY_VILLAGER,
+                        loc.clone().add(0.5, 0.5, 0.5),
+                        10, 0.3, 0.3, 0.3, 0));
             }
         }
 
@@ -385,19 +376,16 @@ public void preGenerateLoot(SpawnerData spawner, LootGenerationCallback callback
         }
 
         try {
-            boolean dataLockAcquired = false;
             try {
-                dataLockAcquired = spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS);
+                if (!spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS)) {
+                    callback.onLootGenerated(Collections.emptyList(), 0);
+                    return;
+                }
             } catch (InterruptedException e) {
                 Thread.currentThread().interrupt();
                 callback.onLootGenerated(Collections.emptyList(), 0);
                 return;
             }
-            
-            if (!dataLockAcquired) {
-                callback.onLootGenerated(Collections.emptyList(), 0);
-                return;
-            }
 
             final int minMobs;
             final int maxMobs;
@@ -465,17 +453,14 @@ public void addPreGeneratedLoot(SpawnerData spawner, List<ItemStack> items, int
             }
 
             try {
-                boolean dataLockAcquired = false;
                 try {
-                    dataLockAcquired = spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS);
+                    if (!spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS)) {
+                        return;
+                    }
                 } catch (InterruptedException e) {
                     Thread.currentThread().interrupt();
                     return;
                 }
-                
-                if (!dataLockAcquired) {
-                    return;
-                }
 
                 final long spawnTime = System.currentTimeMillis();
                 final boolean capacityCheck;

core/src/main/java/github/nighter/smartspawner/spawner/lootgen/SpawnerRangeChecker.java

@@ -10,23 +10,18 @@
 import org.bukkit.entity.Player;
 
 import java.util.*;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
 public class SpawnerRangeChecker {
     private static final long CHECK_INTERVAL = 20L; // 1 second in ticks
     private final SmartSpawner plugin;
     private final SpawnerManager spawnerManager;
-    private final SpawnerLootGenerator spawnerLootGenerator;
-    private final Map<String, Scheduler.Task> spawnerTasks;
     private final ExecutorService executor;
 
     public SpawnerRangeChecker(SmartSpawner plugin) {
         this.plugin = plugin;
         this.spawnerManager = plugin.getSpawnerManager();
-        this.spawnerLootGenerator = plugin.getSpawnerLootGenerator();
-        this.spawnerTasks = new ConcurrentHashMap<>();
         this.executor = Executors.newSingleThreadExecutor(r -> new Thread(r, "SmartSpawner-RangeCheck"));
         initializeRangeCheckTask();
     }
@@ -107,9 +102,10 @@ private boolean isSpawnerValid(SpawnerData spawner) {
     }
 
     private void cleanupRemovedSpawner(String spawnerId) {
-        Scheduler.Task task = spawnerTasks.remove(spawnerId);
-        if (task != null) {
-            task.cancel();
+        // Clear any pre-generated loot when spawner is removed
+        SpawnerData spawner = spawnerManager.getSpawnerById(spawnerId);
+        if (spawner != null) {
+            spawner.clearPreGeneratedLoot();
         }
     }
 
@@ -129,7 +125,7 @@ private void handleSpawnerStateChange(SpawnerData spawner, boolean shouldStop) {
     public void activateSpawner(SpawnerData spawner) {
         deactivateSpawner(spawner);
 
-        // Check if spawner is actually active before starting countdown
+        // Check if spawner is actually active before starting
         if (!spawner.getSpawnerActive()) {
             return;
         }
@@ -138,33 +134,18 @@ public void activateSpawner(SpawnerData spawner) {
         long currentTime = System.currentTimeMillis();
         spawner.setLastSpawnTime(currentTime);
 
-        // Timer manages the spawner active state - actual loot spawning triggered by SpawnerGuiViewManager
-        Scheduler.Task task = Scheduler.runTaskTimer(() -> {
-            // Verify spawner is still active on each tick
-            if (!spawner.getSpawnerActive() || spawner.getSpawnerStop().get()) {
-                return;
-            }
-        }, spawner.getSpawnDelay(), spawner.getSpawnDelay());
-
-        spawnerTasks.put(spawner.getSpawnerId(), task);
-
         // Immediately update any open GUIs to show the countdown
         if (plugin.getSpawnerGuiViewManager().hasViewers(spawner)) {
             plugin.getSpawnerGuiViewManager().updateSpawnerMenuViewers(spawner);
         }
     }
 
     public void deactivateSpawner(SpawnerData spawner) {
-        Scheduler.Task task = spawnerTasks.remove(spawner.getSpawnerId());
-        if (task != null) {
-            task.cancel();
-        }
+        // Clear any pre-generated loot when deactivating
+        spawner.clearPreGeneratedLoot();
     }
 
     public void cleanup() {
-        spawnerTasks.values().forEach(Scheduler.Task::cancel);
-        spawnerTasks.clear();
-
         executor.shutdown();
         try {
             if (!executor.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS)) {
@@ -176,3 +157,4 @@ public void cleanup() {
         }
     }
 }
+