Skip to content

Fix hopper task termination after single execution and refactor for Folia thread-safety #92

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

Merged
ptthanh02 merged 7 commits into from
Nov 1, 2025
Merged

Fix hopper task termination after single execution and refactor for Folia thread-safety #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
246d7ee
Initial plan
Copilot Nov 1, 2025
086f16f
Fix hopper task restart bug and improve thread-safety
Copilot Nov 1, 2025
348a933
Ensure proper cleanup when reloading hopper handler
Copilot Nov 1, 2025
c71d665
Address code review feedback for improved readability
Copilot Nov 1, 2025
b64fc4e
Fix compilation error: use Collection instead of array for getTileEnt…
Copilot Nov 1, 2025
3734aa0
Optimize transferItems() for better performance
Copilot Nov 1, 2025
01980fe
Fix potential exploit by refreshing hopper contents per item
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
9 changes: 8 additions & 1 deletion core/src/main/java/github/nighter/smartspawner/SmartSpawner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,14 @@ private void initializeListeners() {
}

public void setUpHopperHandler() {
this.hopperHandler = getConfig().getBoolean("hopper.enabled", false) ? new HopperHandler(this) : null;
if (this.hopperHandler != null) {
this.hopperHandler.cleanup();
this.hopperHandler = null;
}

if (getConfig().getBoolean("hopper.enabled", false)) {
this.hopperHandler = new HopperHandler(this);
}
}

private void registerListeners() {
Expand Down
171 changes: 117 additions & 54 deletions core/src/main/java/github/nighter/smartspawner/extras/HopperHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,23 @@ public void restartAllHoppers() {
}

private void processChunkHoppers(Chunk chunk) {
if (chunk == null || !chunk.isLoaded()) return;

Location chunkLoc = new Location(chunk.getWorld(),
chunk.getX() * 16 + 8, 64, chunk.getZ() * 16 + 8);

Scheduler.runLocationTask(chunkLoc, () -> {
try {
for (BlockState state : chunk.getTileEntities(block -> block.getType() == Material.HOPPER, false)) {
Collection<BlockState> hopperStates = chunk.getTileEntities(block -> block.getType() == Material.HOPPER, false);
if (hopperStates == null || hopperStates.isEmpty()) return;

for (BlockState state : hopperStates) {
if (state == null) continue;

Block hopperBlock = state.getBlock();
Block aboveBlock = hopperBlock.getRelative(BlockFace.UP);
if (hopperBlock.getType() != Material.HOPPER) continue;

Block aboveBlock = hopperBlock.getRelative(BlockFace.UP);
if (aboveBlock.getType() == Material.SPAWNER) {
startHopperTask(hopperBlock.getLocation(), aboveBlock.getLocation());
}
Expand All @@ -102,15 +110,42 @@ public void onChunkLoad(ChunkLoadEvent event) {

@EventHandler
public void onChunkUnload(ChunkUnloadEvent event) {
if (!plugin.getConfig().getBoolean("hopper.enabled", false)) return;

Chunk chunk = event.getChunk();
for (BlockState state : chunk.getTileEntities(block -> block.getType() == Material.HOPPER, false)) {
stopHopperTask(state.getLocation());
if (chunk == null) return;

try {
Collection<BlockState> hopperStates = chunk.getTileEntities(block -> block.getType() == Material.HOPPER, false);
if (hopperStates == null || hopperStates.isEmpty()) return;

for (BlockState state : hopperStates) {
if (state == null) continue;
stopHopperTask(state.getLocation());
}
} catch (Exception e) {
plugin.getLogger().log(Level.WARNING, "Error stopping hoppers in unloading chunk at " +
chunk.getX() + "," + chunk.getZ(), e);
}
}

public void cleanup() {
activeHoppers.values().forEach(Scheduler.Task::cancel);
activeHoppers.clear();
try {
List<Scheduler.Task> tasks = new ArrayList<>(activeHoppers.values());
activeHoppers.clear();

for (Scheduler.Task task : tasks) {
if (task != null) {
try {
task.cancel();
} catch (Exception e) {
plugin.getLogger().log(Level.WARNING, "Error cancelling hopper task during cleanup", e);
}
}
}
} catch (Exception e) {
plugin.getLogger().log(Level.SEVERE, "Error during hopper handler cleanup", e);
}
}

@EventHandler
Expand All @@ -133,9 +168,10 @@ public void onHopperBreak(BlockBreakEvent event) {

public void startHopperTask(Location hopperLoc, Location spawnerLoc) {
if (!plugin.getConfig().getBoolean("hopper.enabled", false)) return;
if (activeHoppers.containsKey(hopperLoc)) return;
if (hopperLoc == null || spawnerLoc == null) return;

stopHopperTask(hopperLoc);

// Create a runnable for the hopper task
Runnable hopperRunnable = () -> {
try {
if (!isValidSetup(hopperLoc, spawnerLoc)) {
Expand All @@ -145,11 +181,9 @@ public void startHopperTask(Location hopperLoc, Location spawnerLoc) {
transferItems(hopperLoc, spawnerLoc);
} catch (Exception e) {
plugin.getLogger().log(Level.WARNING, "Error in hopper task at " + hopperLoc, e);
// Don't stop the task on error, just log it
}
};

// Use the location-based scheduler for better Folia compatibility
try {
Scheduler.Task task = Scheduler.runLocationTaskTimer(
hopperLoc,
Expand All @@ -158,7 +192,9 @@ public void startHopperTask(Location hopperLoc, Location spawnerLoc) {
plugin.getTimeFromConfig("hopper.check_delay", "3s")
);

activeHoppers.put(hopperLoc, task);
if (task != null && !task.isCancelled()) {
activeHoppers.put(hopperLoc, task);
}
} catch (Exception e) {
plugin.getLogger().log(Level.SEVERE, "Failed to start hopper task at " + hopperLoc, e);
}
Expand All @@ -174,81 +210,108 @@ private boolean isValidSetup(Location hopperLoc, Location spawnerLoc) {
}

public void stopHopperTask(Location hopperLoc) {
if (hopperLoc == null) return;

Scheduler.Task task = activeHoppers.remove(hopperLoc);
if (task != null) {
task.cancel();
try {
task.cancel();
} catch (Exception e) {
plugin.getLogger().log(Level.WARNING, "Error cancelling hopper task at " + hopperLoc, e);
}
}
}

public void restartHopperForSpawner(Location spawnerLoc) {
if (!plugin.getConfig().getBoolean("hopper.enabled", false)) return;

// Find hopper below the spawner
Block spawnerBlock = spawnerLoc.getBlock();
if (spawnerBlock.getType() != Material.SPAWNER) return;

Block hopperBlock = spawnerBlock.getRelative(BlockFace.DOWN);
if (hopperBlock.getType() != Material.HOPPER) return;

Location hopperLoc = hopperBlock.getLocation();

// Stop existing hopper task if any
stopHopperTask(hopperLoc);

// Start new hopper task
startHopperTask(hopperLoc, spawnerLoc);
if (spawnerLoc == null) return;

Scheduler.runLocationTask(spawnerLoc, () -> {
try {
Block spawnerBlock = spawnerLoc.getBlock();
if (spawnerBlock.getType() != Material.SPAWNER) return;

Block hopperBlock = spawnerBlock.getRelative(BlockFace.DOWN);
if (hopperBlock.getType() != Material.HOPPER) return;

Location hopperLoc = hopperBlock.getLocation();

startHopperTask(hopperLoc, spawnerLoc);
} catch (Exception e) {
plugin.getLogger().log(Level.WARNING, "Error restarting hopper for spawner at " + spawnerLoc, e);
}
});
}

private void transferItems(Location hopperLoc, Location spawnerLoc) {
if (hopperLoc == null || spawnerLoc == null) return;

SpawnerData spawner = spawnerManager.getSpawnerByLocation(spawnerLoc);
if (spawner == null) return;

// Use inventoryLock for hopper transfers
ReentrantLock lock = spawner.getInventoryLock();
if (!lock.tryLock()) return; // Skip this tick if we can't get the lock
if (!lock.tryLock()) return;

try {
Block hopperBlock = hopperLoc.getBlock();
if (hopperBlock.getType() != Material.HOPPER) {
stopHopperTask(hopperLoc);
return;
}

VirtualInventory virtualInv = spawner.getVirtualInventory();
Hopper hopper = (Hopper) hopperLoc.getBlock().getState(false);
if (virtualInv == null) return;

Hopper hopper = (Hopper) hopperBlock.getState(false);
if (hopper == null) return;

Map<Integer, ItemStack> displayItems = virtualInv.getDisplayInventory();
if (displayItems == null || displayItems.isEmpty()) return;

int itemsPerTransfer = plugin.getConfig().getInt("hopper.stack_per_transfer", 5);

org.bukkit.inventory.Inventory hopperInv = hopper.getInventory();

List<ItemStack> itemsToRemove = new ArrayList<>(itemsPerTransfer);
int transferred = 0;
boolean inventoryChanged = false;

Map<Integer, ItemStack> displayItems = virtualInv.getDisplayInventory();
List<ItemStack> itemsToRemove = new ArrayList<>();

for (Map.Entry<Integer, ItemStack> entry : displayItems.entrySet()) {
if (transferred >= itemsPerTransfer) break;

ItemStack item = entry.getValue();
if (item == null || item.getType() == Material.AIR) continue;

ItemStack[] hopperContents = hopper.getInventory().getContents();
for (int i = 0; i < hopperContents.length; i++) {
if (transferred >= itemsPerTransfer) break;

ItemStack[] hopperContents = hopperInv.getContents();

boolean itemTransferred = false;
for (int i = 0; i < hopperContents.length && !itemTransferred; i++) {
ItemStack hopperItem = hopperContents[i];

if (hopperItem == null || hopperItem.getType() == Material.AIR) {
hopper.getInventory().setItem(i, item.clone());
hopperInv.setItem(i, item.clone());
itemsToRemove.add(item);
transferred++;
inventoryChanged = true;
break;
} else if (hopperItem.isSimilar(item) &&
hopperItem.getAmount() < hopperItem.getMaxStackSize()) {
int space = hopperItem.getMaxStackSize() - hopperItem.getAmount();
int toTransfer = Math.min(space, item.getAmount());

hopperItem.setAmount(hopperItem.getAmount() + toTransfer);

ItemStack toRemove = item.clone();
toRemove.setAmount(toTransfer);
itemsToRemove.add(toRemove);

transferred++;
inventoryChanged = true;
break;
itemTransferred = true;
} else if (hopperItem.isSimilar(item)) {
int currentAmount = hopperItem.getAmount();
int maxStackSize = hopperItem.getMaxStackSize();

if (currentAmount < maxStackSize) {
int space = maxStackSize - currentAmount;
int toTransfer = Math.min(space, item.getAmount());

hopperItem.setAmount(currentAmount + toTransfer);

ItemStack toRemove = item.clone();
toRemove.setAmount(toTransfer);
itemsToRemove.add(toRemove);

transferred++;
inventoryChanged = true;
itemTransferred = true;
}
}
}
}
Expand All @@ -261,7 +324,7 @@ private void transferItems(Location hopperLoc, Location spawnerLoc) {
updateOpenGuis(spawner);
}
} catch (Exception e) {
plugin.getLogger().log(Level.WARNING, "Error transferring items from spawner to hopper", e);
plugin.getLogger().log(Level.WARNING, "Error transferring items from spawner to hopper at " + hopperLoc, e);
} finally {
lock.unlock();
}
Expand Down