diff --git a/design/log.md b/design/log.md index c494e56..cbcf907 100644 --- a/design/log.md +++ b/design/log.md @@ -30,3 +30,10 @@ description: Chronological record of material decisions affecting the Spigot Har - Implemented deterministic connected-crop traversal, one-operation-per-player scheduling, one-crop-per-tick harvesting, protection-event checks, automatic replanting, inventory delivery, and overflow drops. - Added player status and boss-bar controls, five-second idle progress presentation, unlock titles, administrative inspection and mutations, offline-player lookup, and safe confirmed resets. - Developed the progression, traversal, drops, persistence, settings, status, and administration behavior through failing-first automated tests. + +## 2026-08-14 — Triggering crop replant fix + +- Changed auto-harvest to process and replant the manually broken triggering crop immediately before animating connected crops. +- Kept the triggering crop inside the level cap, so Level I processes the trigger and at most three connected crops. +- Added regression coverage for triggering-crop execution, replanting, and operation-cap accounting. +- Verified the fix with `./gradlew clean check jar`. diff --git a/design/user-stories/us-003-harvest-connected-mature-crops.md b/design/user-stories/us-003-harvest-connected-mature-crops.md index 1368055..749a67e 100644 --- a/design/user-stories/us-003-harvest-connected-mature-crops.md +++ b/design/user-stories/us-003-harvest-connected-mature-crops.md @@ -13,7 +13,7 @@ As a **player**, I want one normal crop break to harvest a connected area at a v - [x] Normally breaking a fully grown supported crop starts auto-harvest when the player has at least Level I for that crop. - [x] Breaking while sneaking performs normal single-crop harvesting and never starts auto-harvest. -- [x] The manually broken triggering crop is included in the operation's maximum crop count. +- [x] The manually broken triggering crop is included in the operation's maximum crop count, so Level I processes the trigger plus no more than three connected crops. - [x] Auto-harvest traverses only fully grown crops of the same type as the triggering crop. - [x] A crop is connected when its horizontal position touches the current crop in any of the eight directions and its Y coordinate differs from the current crop by no more than one block. - [x] Connectivity is evaluated per link, allowing a connected operation to follow gradual rises and drops. diff --git a/design/user-stories/us-004-replant-crops-and-distribute-drops.md b/design/user-stories/us-004-replant-crops-and-distribute-drops.md index 6185555..bae8b19 100644 --- a/design/user-stories/us-004-replant-crops-and-distribute-drops.md +++ b/design/user-stories/us-004-replant-crops-and-distribute-drops.md @@ -11,6 +11,8 @@ As a **player**, I want harvested crops replanted and their useful drops deliver ## Acceptance criteria +- [x] The manually broken crop that starts auto-harvest is harvested and replanted exactly once as the first crop in the operation. +- [x] Automated regression tests cover the triggering-crop planning, cap accounting, and replant path. - [x] Every crop successfully harvested by an auto-harvest operation is immediately replanted as the same crop at its minimum growth stage. - [x] Replanting consumes one appropriate planting item from that crop's generated drops before remaining drops are delivered. - [x] Wheat consumes one wheat seed, carrots consume one carrot, potatoes consume one potato, and beetroot consumes one beetroot seed when replanted. diff --git a/src/main/java/games/dmg/spigotharvest/HarvestListener.java b/src/main/java/games/dmg/spigotharvest/HarvestListener.java index 18c6656..69331f3 100644 --- a/src/main/java/games/dmg/spigotharvest/HarvestListener.java +++ b/src/main/java/games/dmg/spigotharvest/HarvestListener.java @@ -51,9 +51,9 @@ public final class HarvestListener implements Listener { return; } int cap = settings.progression(crop).capForLevel(level); - if (operations.start(player, block, crop, cap)) { - event.setCancelled(true); - } else { + event.setCancelled(true); + if (!operations.start(player, block, crop, cap)) { + event.setCancelled(false); progress.record(player, crop, 1); } } diff --git a/src/main/java/games/dmg/spigotharvest/HarvestOperationManager.java b/src/main/java/games/dmg/spigotharvest/HarvestOperationManager.java index 3cb7e64..0aa3afe 100644 --- a/src/main/java/games/dmg/spigotharvest/HarvestOperationManager.java +++ b/src/main/java/games/dmg/spigotharvest/HarvestOperationManager.java @@ -38,18 +38,38 @@ public final class HarvestOperationManager { task = Bukkit.getScheduler().runTaskTimer(plugin, this::tick, 1, 1); } + HarvestOperationManager( + HarvestSettings settings, + HarvestProgressService progress, + BukkitTask task + ) { + this.settings = settings; + this.progress = progress; + this.task = task; + } + public boolean start(Player player, Block origin, CropType crop, int cap) { if (operations.containsKey(player.getUniqueId())) { return false; } World world = origin.getWorld(); BlockPosition start = position(origin); - List crops = ConnectedCropSearch.find( - start, candidate -> isMatureLoaded(world, candidate, crop), cap); - if (crops.isEmpty()) { + final HarvestOperationPlan plan; + try { + plan = HarvestOperationPlan.create( + start, candidate -> isMatureLoaded(world, candidate, crop), cap); + } catch (IllegalArgumentException exception) { return false; } - operations.put(player.getUniqueId(), new Operation(world.getUID(), crop, crops)); + Operation operation = new Operation(world.getUID(), crop, plan.connectedCrops()); + operations.put(player.getUniqueId(), operation); + if (!harvest(player, world, crop, plan.trigger(), false)) { + operations.remove(player.getUniqueId()); + return false; + } + if (!operation.hasNext()) { + operations.remove(player.getUniqueId()); + } return true; } @@ -88,7 +108,7 @@ public final class HarvestOperationManager { } int processed = 0; while (processed < settings.cropsPerTick() && operation.hasNext()) { - harvest(player, world, operation.crop(), operation.next()); + harvest(player, world, operation.crop(), operation.next(), true); processed++; } if (!operation.hasNext()) { @@ -97,25 +117,33 @@ public final class HarvestOperationManager { } } - private void harvest(Player player, World world, CropType crop, BlockPosition position) { + private boolean harvest( + Player player, + World world, + CropType crop, + BlockPosition position, + boolean dispatchProtectionEvent + ) { if (!isMatureLoaded(world, position, crop)) { - return; + return false; } Block block = world.getBlockAt(position.x(), position.y(), position.z()); - BlockBreakEvent event = new BlockBreakEvent(block, player); - dispatchingBreakEvent.add(player.getUniqueId()); - try { - Bukkit.getPluginManager().callEvent(event); - } finally { - dispatchingBreakEvent.remove(player.getUniqueId()); - } - if (event.isCancelled()) { - return; + if (dispatchProtectionEvent) { + BlockBreakEvent event = new BlockBreakEvent(block, player); + dispatchingBreakEvent.add(player.getUniqueId()); + try { + Bukkit.getPluginManager().callEvent(event); + } finally { + dispatchingBreakEvent.remove(player.getUniqueId()); + } + if (event.isCancelled()) { + return false; + } } Collection generated = block.getDrops(player.getInventory().getItemInMainHand(), player); CropDropPlan plan = CropDropPlan.create(crop, generated); if (!plan.canReplant()) { - return; + return false; } block.setType(Material.AIR, false); block.setType(crop.blockMaterial(), false); @@ -125,6 +153,7 @@ public final class HarvestOperationManager { } deliver(player, plan.remainingDrops()); progress.record(player, crop, 1); + return true; } private static void deliver(Player player, List drops) { diff --git a/src/main/java/games/dmg/spigotharvest/HarvestOperationPlan.java b/src/main/java/games/dmg/spigotharvest/HarvestOperationPlan.java new file mode 100644 index 0000000..3f2cd7e --- /dev/null +++ b/src/main/java/games/dmg/spigotharvest/HarvestOperationPlan.java @@ -0,0 +1,30 @@ +package games.dmg.spigotharvest; + +import java.util.List; +import java.util.function.Predicate; + +/** A bounded operation split into its immediate trigger and animated remainder. */ +public record HarvestOperationPlan( + BlockPosition trigger, + List connectedCrops +) { + public HarvestOperationPlan { + connectedCrops = List.copyOf(connectedCrops); + } + + public static HarvestOperationPlan create( + BlockPosition trigger, + Predicate eligible, + int maximumCrops + ) { + List crops = ConnectedCropSearch.find(trigger, eligible, maximumCrops); + if (crops.isEmpty()) { + throw new IllegalArgumentException("trigger must be an eligible crop"); + } + return new HarvestOperationPlan(crops.get(0), crops.subList(1, crops.size())); + } + + public int totalCropCount() { + return 1 + connectedCrops.size(); + } +} diff --git a/src/test/java/games/dmg/spigotharvest/HarvestOperationPlanTest.java b/src/test/java/games/dmg/spigotharvest/HarvestOperationPlanTest.java new file mode 100644 index 0000000..d651408 --- /dev/null +++ b/src/test/java/games/dmg/spigotharvest/HarvestOperationPlanTest.java @@ -0,0 +1,28 @@ +package games.dmg.spigotharvest; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Set; +import org.junit.jupiter.api.Test; + +final class HarvestOperationPlanTest { + @Test + void levelOneProcessesTriggerFirstAndOnlyThreeConnectedCrops() { + BlockPosition trigger = new BlockPosition(0, 64, 0); + Set mature = Set.of( + trigger, + new BlockPosition(1, 64, 0), + new BlockPosition(0, 64, 1), + new BlockPosition(-1, 64, 0), + new BlockPosition(0, 64, -1) + ); + + HarvestOperationPlan plan = HarvestOperationPlan.create(trigger, mature::contains, 4); + + assertEquals(trigger, plan.trigger()); + assertEquals(3, plan.connectedCrops().size()); + assertTrue(!plan.connectedCrops().contains(trigger)); + assertEquals(4, plan.totalCropCount()); + } +} diff --git a/src/test/java/games/dmg/spigotharvest/TriggerCropReplantTest.java b/src/test/java/games/dmg/spigotharvest/TriggerCropReplantTest.java new file mode 100644 index 0000000..0aaa573 --- /dev/null +++ b/src/test/java/games/dmg/spigotharvest/TriggerCropReplantTest.java @@ -0,0 +1,67 @@ +package games.dmg.spigotharvest; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.UUID; +import org.bukkit.Material; +import org.bukkit.World; +import org.bukkit.block.Block; +import org.bukkit.block.data.Ageable; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.scheduler.BukkitTask; +import org.junit.jupiter.api.Test; + +final class TriggerCropReplantTest { + @Test + void startImmediatelyHarvestsAndReplantsTriggerExactlyOnce() { + HarvestProgressService progress = mock(HarvestProgressService.class); + BukkitTask task = mock(BukkitTask.class); + HarvestOperationManager manager = new HarvestOperationManager( + HarvestSettings.defaults(), progress, task); + Player player = mock(Player.class); + PlayerInventory inventory = mock(PlayerInventory.class); + World world = mock(World.class); + Block trigger = mock(Block.class); + Block air = mock(Block.class); + Ageable ageable = mock(Ageable.class); + UUID playerId = UUID.randomUUID(); + + when(player.getUniqueId()).thenReturn(playerId); + when(player.getInventory()).thenReturn(inventory); + when(inventory.getItemInMainHand()).thenReturn(new ItemStack(Material.AIR)); + when(world.getUID()).thenReturn(UUID.randomUUID()); + when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); + when(world.getBlockAt(anyInt(), anyInt(), anyInt())) + .thenAnswer(invocation -> invocation.getArgument(0, Integer.class) == 0 + && invocation.getArgument(1, Integer.class) == 64 + && invocation.getArgument(2, Integer.class) == 0 ? trigger : air); + when(air.getType()).thenReturn(Material.AIR); + when(trigger.getWorld()).thenReturn(world); + when(trigger.getX()).thenReturn(0); + when(trigger.getY()).thenReturn(64); + when(trigger.getZ()).thenReturn(0); + when(trigger.getType()).thenReturn(Material.WHEAT); + when(trigger.getBlockData()).thenReturn(ageable); + when(ageable.getAge()).thenReturn(7); + when(ageable.getMaximumAge()).thenReturn(7); + when(trigger.getDrops(any(ItemStack.class), any(Player.class))).thenReturn(List.of( + new ItemStack(Material.WHEAT), new ItemStack(Material.WHEAT_SEEDS, 2))); + when(progress.record(player, CropType.WHEAT, 1)).thenReturn(true); + + assertTrue(manager.start(player, trigger, CropType.WHEAT, 4)); + + verify(trigger).setType(Material.AIR, false); + verify(trigger).setType(Material.WHEAT, false); + verify(ageable).setAge(0); + verify(trigger).setBlockData(ageable, false); + verify(progress).record(player, CropType.WHEAT, 1); + } +}