diff --git a/pom.xml b/pom.xml index 630a18b..f276d56 100644 --- a/pom.xml +++ b/pom.xml @@ -67,7 +67,7 @@ -LOCAL - 1.27.1 + 1.27.2 BentoBoxWorld_AOneBlock bentobox-world diff --git a/src/main/java/world/bentobox/aoneblock/listeners/BlockListener.java b/src/main/java/world/bentobox/aoneblock/listeners/BlockListener.java index 6c006e6..8fc6384 100644 --- a/src/main/java/world/bentobox/aoneblock/listeners/BlockListener.java +++ b/src/main/java/world/bentobox/aoneblock/listeners/BlockListener.java @@ -423,7 +423,7 @@ private void process(@NonNull Cancellable e, @NonNull Island island, @Nullable P // Process phase changes and requirements ProcessPhaseResult phaseResult = processPhase(e, island, is, player, world, block); - if (e.isCancelled()) { + if (phaseResult == null || e.isCancelled()) { return; } @@ -455,16 +455,29 @@ private record ProcessPhaseResult(OneBlockPhase phase, boolean isCurrPhaseNew, i * @param player - player involved * @param world - world where processing occurs * @param block - block being processed - * @return ProcessPhaseResult containing phase details + * @return ProcessPhaseResult containing phase details, or null if no phase covers the + * island's block number. In that case the event is cancelled so the magic block + * is not lost, and an error is logged for the admin. */ + @Nullable private ProcessPhaseResult processPhase(Cancellable e, Island i, OneBlockIslands is, Player player, World world, Block block) { OneBlockPhase phase = oneBlocksManager.getPhase(is.getBlockNumber()); String prevPhaseName = is.getPhaseName(); - if (Objects.requireNonNull(phase).getGotoBlock() != null) { + if (phase != null && phase.getGotoBlock() != null) { phase = handleGoto(is, phase.getGotoBlock()); } + if (phase == null) { + // No phase covers this block number. This happens when no phase files loaded, + // when the first phase does not start at block 0, or when a goto points below + // the first phase. Keep the block in place rather than breaking the island. + addon.logError("No phase found for block number " + is.getBlockNumber() + " on island " + + i.getUniqueId() + ". Check the phase files in the phases folder."); + e.setCancelled(true); + return null; + } + String currPhaseName = phase.getPhaseName() == null ? "" : phase.getPhaseName(); handlePhaseChange(is, currPhaseName); @@ -516,7 +529,7 @@ private void handleNewPhase(Player player, Island i, OneBlockIslands is, OneBloc */ private void handlePhaseChange(OneBlockIslands is, String currPhaseName) { OneBlockPhase nextPhase = oneBlocksManager.getPhase(is.getBlockNumber() + 1); - if (Objects.requireNonNull(nextPhase).getGotoBlock() != null) { + if (nextPhase != null && nextPhase.getGotoBlock() != null) { nextPhase = oneBlocksManager.getPhase(nextPhase.getGotoBlock()); } String nextPhaseName = nextPhase == null || nextPhase.getPhaseName() == null ? "" : nextPhase.getPhaseName(); diff --git a/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest.java b/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest.java index fcd087c..786a48d 100644 --- a/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest.java +++ b/src/test/java/world/bentobox/aoneblock/listeners/BlockListenerTest.java @@ -3,6 +3,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -17,7 +19,10 @@ import org.bukkit.World; import org.bukkit.block.Block; import org.bukkit.block.BlockFace; +import org.bukkit.entity.ArmorStand; +import org.bukkit.entity.EntityType; import org.bukkit.event.block.BlockFromToEvent; +import org.bukkit.event.entity.EntityInteractEvent; import org.eclipse.jdt.annotation.NonNull; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -192,6 +197,57 @@ void testOnBlockFromToCenterBlock() { assertTrue(e.isCancelled()); } + /** + * When no phase covers the island's block number (e.g. phase files failed to load or + * a goto points below the first phase), the break must be cancelled and an error logged + * instead of throwing a NullPointerException. + */ + @Test + void testProcessNoPhaseForBlockNumberCancelsAndLogs() { + when(addon.inWorld(any(World.class))).thenReturn(true); + island.setCenter(location); + when(im.getIslandAt(location)).thenReturn(Optional.of(island)); + when(obm.getPhase(anyInt())).thenReturn(null); + + Block block = mock(Block.class); + when(block.getLocation()).thenReturn(location); + when(block.getWorld()).thenReturn(world); + ArmorStand minion = mock(ArmorStand.class); + when(minion.getType()).thenReturn(EntityType.ARMOR_STAND); + + EntityInteractEvent e = new EntityInteractEvent(minion, block); + bl.onBlockBreakByMinion(e); + + assertTrue(e.isCancelled()); + verify(addon).logError(anyString()); + } + + /** + * A phase whose goto target lies below the first phase must also be handled gracefully. + */ + @Test + void testProcessGotoBelowFirstPhaseCancelsAndLogs() { + when(addon.inWorld(any(World.class))).thenReturn(true); + island.setCenter(location); + when(im.getIslandAt(location)).thenReturn(Optional.of(island)); + OneBlockPhase gotoPhase = new OneBlockPhase("0"); + gotoPhase.setGotoBlock(-5); + when(obm.getPhase(0)).thenReturn(gotoPhase); + when(obm.getPhase(-5)).thenReturn(null); + + Block block = mock(Block.class); + when(block.getLocation()).thenReturn(location); + when(block.getWorld()).thenReturn(world); + ArmorStand minion = mock(ArmorStand.class); + when(minion.getType()).thenReturn(EntityType.ARMOR_STAND); + + EntityInteractEvent e = new EntityInteractEvent(minion, block); + bl.onBlockBreakByMinion(e); + + assertTrue(e.isCancelled()); + verify(addon).logError(anyString()); + } + /** * Test method for {@link world.bentobox.aoneblock.listeners.BlockListener#saveCache()}. */