Skip to content

Refactor loot generation system to prevent skipped cycles #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 5 commits into from
Closed

Refactor loot generation system to prevent skipped cycles #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
900cf49
Initial plan
Copilot Nov 1, 2025
b164c5c
Refactor loot generation: remove second lock and clean up code
Copilot Nov 1, 2025
57b43bb
Fix skipped cycle after stack size update by resetting lastSpawnTime
Copilot Nov 1, 2025
e1e9509
Use blocking lock for dataLock to prevent skipped cycles during stack…
Copilot Nov 1, 2025
095dbfc
Remove timer reset from setStackSize to prevent skipped loot cycles
Copilot Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 37 additions & 114 deletions core/src/main/java/github/nighter/smartspawner/spawner/lootgen/SpawnerLootGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,58 +29,47 @@ public SpawnerLootGenerator(SmartSpawner plugin) {
}

public LootResult generateLoot(int minMobs, int maxMobs, SpawnerData spawner) {

int mobCount = random.nextInt(maxMobs - minMobs + 1) + minMobs;
int totalExperience = spawner.getEntityExperienceValue() * mobCount;

// Get valid items from the spawner's EntityLootConfig
List<LootItem> validItems = spawner.getValidLootItems();
List<LootItem> validItems = spawner.getValidLootItems();

if (validItems.isEmpty()) {
return new LootResult(Collections.emptyList(), totalExperience);
}

// Use a Map to consolidate identical drops instead of List
Map<ItemStack, Integer> consolidatedLoot = new HashMap<>();

// Process mobs in batch rather than individually
for (LootItem lootItem : validItems) {
// Calculate the probability for the entire mob batch at once
int successfulDrops = 0;

// Calculate binomial distribution - how many mobs will drop this item
for (int i = 0; i < mobCount; i++) {
if (random.nextDouble() * 100 <= lootItem.getChance()) {
successfulDrops++;
}
}

if (successfulDrops > 0) {
// Create item just once per loot type
ItemStack prototype = lootItem.createItemStack(random);
if (prototype != null) {
// Total amount across all mobs
int totalAmount = 0;
for (int i = 0; i < successfulDrops; i++) {
totalAmount += lootItem.generateAmount(random);
}

if (totalAmount > 0) {
// Add to consolidated map
consolidatedLoot.merge(prototype, totalAmount, Integer::sum);
}
}
}
}

// Convert consolidated map to item stacks
List<ItemStack> finalLoot = new ArrayList<>(consolidatedLoot.size());
for (Map.Entry<ItemStack, Integer> entry : consolidatedLoot.entrySet()) {
ItemStack item = entry.getKey().clone();
item.setAmount(Math.min(entry.getValue(), item.getMaxStackSize()));
finalLoot.add(item);

// Handle amounts exceeding max stack size
int remaining = entry.getValue() - item.getMaxStackSize();
while (remaining > 0) {
ItemStack extraStack = item.clone();
Expand All @@ -94,18 +83,12 @@ public LootResult generateLoot(int minMobs, int maxMobs, SpawnerData spawner) {
}

public void spawnLootToSpawner(SpawnerData spawner) {
// Try to acquire the lock, but don't block if it's already locked
// This ensures we don't block the server thread while waiting for the lock
boolean lockAcquired = spawner.getLootGenerationLock().tryLock();
if (!lockAcquired) {
// Lock is already held, which means another loot generation is happening
// Skip this loot generation cycle
return;
}

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);
Expand All @@ -115,17 +98,13 @@ public void spawnLootToSpawner(SpawnerData spawner) {
}

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;

Expand All @@ -137,115 +116,78 @@ public void spawnLootToSpawner(SpawnerData spawner) {
return;
}

// Get exact inventory slot usage
usedSlots = new AtomicInteger(spawner.getVirtualInventory().getUsedSlots());
maxSlots = new AtomicInteger(spawner.getMaxSpawnerLootSlots());

// Check if both inventory and exp are full, only then skip loot generation
if (usedSlots.get() >= maxSlots.get() && spawner.getSpawnerExp() >= spawner.getMaxStoredExp()) {
if (!spawner.getIsAtCapacity()) {
spawner.setIsAtCapacity(true);
}
return; // Skip generation if both exp and inventory are full
return;
}

// 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 {
spawner.getDataLock().unlock();
}

// Run heavy calculations async and batch updates using the Scheduler
Scheduler.runTaskAsync(() -> {
// Generate loot with full mob count
LootResult loot = generateLoot(minMobs, maxMobs, spawner);

// Only proceed if we generated something
if (loot.getItems().isEmpty() && loot.getExperience() == 0) {
return;
}

// Switch back to main thread for Bukkit API calls using location-aware scheduling
Scheduler.runLocationTask(spawner.getSpawnerLocation(), () -> {
// Re-acquire the lock for the update phase
// This ensures the spawner hasn't been modified (like stack size changes)
// between our async calculations and now
boolean updateLockAcquired = spawner.getLootGenerationLock().tryLock();
if (!updateLockAcquired) {
// Lock is held, stack size is changing, skip this update
return;
}
boolean changed = false;

try {
// Modified approach: Handle items and exp separately
boolean changed = false;
if (loot.getExperience() > 0 && spawner.getSpawnerExp() < spawner.getMaxStoredExp()) {
int currentExp = spawner.getSpawnerExp();
int maxExp = spawner.getMaxStoredExp();
int newExp = Math.min(currentExp + loot.getExperience(), maxExp);

// Process experience if there's any to add and not at max
if (loot.getExperience() > 0 && spawner.getSpawnerExp() < spawner.getMaxStoredExp()) {
int currentExp = spawner.getSpawnerExp();
int maxExp = spawner.getMaxStoredExp();
int newExp = Math.min(currentExp + loot.getExperience(), maxExp);

if (newExp != currentExp) {
spawner.setSpawnerExp(newExp);
changed = true;
}
if (newExp != currentExp) {
spawner.setSpawnerExp(newExp);
changed = true;
}
}

// Re-check max slots as it could have changed
maxSlots.set(spawner.getMaxSpawnerLootSlots());
usedSlots.set(spawner.getVirtualInventory().getUsedSlots());

// Process items if there are any to add and inventory isn't completely full
if (!loot.getItems().isEmpty() && usedSlots.get() < maxSlots.get()) {
List<ItemStack> itemsToAdd = new ArrayList<>(loot.getItems());
maxSlots.set(spawner.getMaxSpawnerLootSlots());
usedSlots.set(spawner.getVirtualInventory().getUsedSlots());

// Get exact calculation of slots with the new items
int totalRequiredSlots = calculateRequiredSlots(itemsToAdd, spawner.getVirtualInventory());
if (!loot.getItems().isEmpty() && usedSlots.get() < maxSlots.get()) {
List<ItemStack> itemsToAdd = new ArrayList<>(loot.getItems());

// If we'll exceed the limit, limit the items we're adding
if (totalRequiredSlots > maxSlots.get()) {
itemsToAdd = limitItemsToAvailableSlots(itemsToAdd, spawner);
}
int totalRequiredSlots = calculateRequiredSlots(itemsToAdd, spawner.getVirtualInventory());

if (!itemsToAdd.isEmpty()) {
spawner.addItemsAndUpdateSellValue(itemsToAdd);
changed = true;
}
if (totalRequiredSlots > maxSlots.get()) {
itemsToAdd = limitItemsToAvailableSlots(itemsToAdd, spawner);
}

if (!changed) {
return;
if (!itemsToAdd.isEmpty()) {
spawner.addItemsAndUpdateSellValue(itemsToAdd);
changed = true;
}
}

// Update spawn time only after successful loot addition
// This prevents skipped spawns when the lock fails
// Must acquire dataLock to safely update lastSpawnTime
boolean updateDataLockAcquired = spawner.getDataLock().tryLock();
if (updateDataLockAcquired) {
try {
spawner.setLastSpawnTime(spawnTime);
} finally {
spawner.getDataLock().unlock();
}
}

// Check if spawner is now at capacity and update status if needed
spawner.updateCapacityStatus();

// Handle GUI updates in batches
handleGuiUpdates(spawner);
if (!changed) {
return;
}

// Mark for saving only once
spawnerManager.markSpawnerModified(spawner.getSpawnerId());
} finally {
spawner.getLootGenerationLock().unlock();
boolean updateDataLockAcquired = spawner.getDataLock().tryLock();
if (updateDataLockAcquired) {
try {
spawner.setLastSpawnTime(spawnTime);
} finally {
spawner.getDataLock().unlock();
}
}

spawner.updateCapacityStatus();
handleGuiUpdates(spawner);
spawnerManager.markSpawnerModified(spawner.getSpawnerId());
});
});
} finally {
Expand All @@ -257,55 +199,43 @@ private List<ItemStack> limitItemsToAvailableSlots(List<ItemStack> items, Spawne
VirtualInventory currentInventory = spawner.getVirtualInventory();
int maxSlots = spawner.getMaxSpawnerLootSlots();

// If already full, return empty list
if (currentInventory.getUsedSlots() >= maxSlots) {
return Collections.emptyList();
}

// Create a simulation inventory
Map<VirtualInventory.ItemSignature, Long> simulatedInventory = new HashMap<>(currentInventory.getConsolidatedItems());
List<ItemStack> acceptedItems = new ArrayList<>();

// Sort items by priority (you can change this sorting strategy)
items.sort(Comparator.comparing(item -> item.getType().name()));

for (ItemStack item : items) {
if (item == null || item.getAmount() <= 0) continue;

// Add to simulation and check slot count
Map<VirtualInventory.ItemSignature, Long> tempSimulation = new HashMap<>(simulatedInventory);
VirtualInventory.ItemSignature sig = new VirtualInventory.ItemSignature(item);
tempSimulation.merge(sig, (long) item.getAmount(), Long::sum);

// Calculate slots needed
int slotsNeeded = calculateSlots(tempSimulation);

// If we still have room, accept this item
if (slotsNeeded <= maxSlots) {
acceptedItems.add(item);
simulatedInventory = tempSimulation; // Update simulation
simulatedInventory = tempSimulation;
} else {
// Try to accept a partial amount of this item
int maxStackSize = item.getMaxStackSize();
long currentAmount = simulatedInventory.getOrDefault(sig, 0L);

// Calculate how many we can add without exceeding slot limit
int remainingSlots = maxSlots - calculateSlots(simulatedInventory);
if (remainingSlots > 0) {
// Maximum items we can add in the remaining slots
long maxAddAmount = remainingSlots * maxStackSize - (currentAmount % maxStackSize);
if (maxAddAmount > 0) {
// Create a partial item
ItemStack partialItem = item.clone();
partialItem.setAmount((int) Math.min(maxAddAmount, item.getAmount()));
acceptedItems.add(partialItem);

// Update simulation
simulatedInventory.merge(sig, (long) partialItem.getAmount(), Long::sum);
}
}

// We've filled all slots, stop processing
break;
}
}
Expand All @@ -314,40 +244,33 @@ private List<ItemStack> limitItemsToAvailableSlots(List<ItemStack> items, Spawne
}

private int calculateSlots(Map<VirtualInventory.ItemSignature, Long> items) {
// Use a more efficient calculation approach
return items.entrySet().stream()
.mapToInt(entry -> {
long amount = entry.getValue();
int maxStackSize = entry.getKey().getTemplateRef().getMaxStackSize();
// Use integer division with ceiling function
return (int) ((amount + maxStackSize - 1) / maxStackSize);
})
.sum();
}

private int calculateRequiredSlots(List<ItemStack> items, VirtualInventory inventory) {
// Create a temporary map to simulate how items would stack
Map<VirtualInventory.ItemSignature, Long> simulatedItems = new HashMap<>();

// First, get existing items if we need to account for them
if (inventory != null) {
simulatedItems.putAll(inventory.getConsolidatedItems());
}

// Add the new items to our simulation
for (ItemStack item : items) {
if (item == null || item.getAmount() <= 0) continue;

VirtualInventory.ItemSignature sig = new VirtualInventory.ItemSignature(item);
simulatedItems.merge(sig, (long) item.getAmount(), Long::sum);
}

// Calculate exact slots needed
return calculateSlots(simulatedItems);
}

private void handleGuiUpdates(SpawnerData spawner) {
// Show particles if needed
if (plugin.getConfig().getBoolean("particle.spawner_generate_loot", true)) {
Location loc = spawner.getSpawnerLocation();
World world = loc.getWorld();
Expand Down
2 changes: 0 additions & 2 deletions core/src/main/java/github/nighter/smartspawner/spawner/properties/SpawnerData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,6 @@ private void updateStackSize(int newStackSize, boolean restartHopper) {
transferItemsToNewInventory(currentItems, newInventory);
this.virtualInventory = newInventory;

// Reset lastSpawnTime to prevent exploit where players break spawners to trigger immediate loot
this.lastSpawnTime = System.currentTimeMillis();
updateHologramData();

// Invalidate GUI cache when stack size changes
Expand Down