Reworked how new island spots are found.

Fixed bug where max attempts check was not working, which could cause a
timeout crash.

https://github.com/BentoBoxWorld/BentoBox/issues/1057
This commit is contained in:
tastybento 2019-12-15 14:41:59 -08:00
parent 30abc0e6a8
commit c3442c29ba
8 changed files with 67 additions and 74 deletions

View File

@ -1,6 +1,5 @@
package world.bentobox.bentobox.api.commands.island;
import java.io.IOException;
import java.util.List;
import org.eclipse.jdt.annotation.Nullable;
@ -94,9 +93,9 @@ public class IslandCreateCommand extends CompositeCommand {
.reason(Reason.CREATE)
.name(name)
.build();
} catch (IOException e) {
} catch (Exception e) {
getPlugin().logError("Could not create island for player. " + e.getMessage());
user.sendMessage("commands.island.create.unable-create-island");
user.sendMessage(e.getMessage());
return false;
}
if (getSettings().isResetCooldownOnCreate()) {

View File

@ -1,6 +1,5 @@
package world.bentobox.bentobox.api.commands.island;
import java.io.IOException;
import java.util.List;
import org.bukkit.Bukkit;
@ -140,9 +139,9 @@ public class IslandResetCommand extends ConfirmableCommand {
.name(name);
if (noPaste) builder.noPaste();
builder.build();
} catch (IOException e) {
} catch (Exception e) {
getPlugin().logError("Could not create island for player. " + e.getMessage());
user.sendMessage("commands.island.create.unable-create-island");
user.sendMessage(e.getMessage());
return false;
}
setCooldown(user.getUniqueId(), getSettings().getResetCooldown());

View File

@ -1,5 +1,6 @@
package world.bentobox.bentobox.managers.island;
import java.util.Arrays;
import java.util.EnumMap;
import java.util.HashSet;
import java.util.Map;
@ -8,6 +9,7 @@ import java.util.Set;
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.World;
import org.bukkit.block.BlockFace;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.util.Util;
@ -24,14 +26,15 @@ public class DefaultNewIslandLocationStrategy implements NewIslandLocationStrate
* The amount times to tolerate island check returning blocks without kwnon
* island.
*/
protected static final Integer MAX_UNOWNED_ISLANDS = 10;
protected static final Integer MAX_UNOWNED_ISLANDS = 20;
protected enum Result {
ISLAND_FOUND, BLOCK_AT_CENTER, BLOCKS_IN_AREA, FREE
ISLAND_FOUND, BLOCKS_IN_AREA, FREE
}
protected BentoBox plugin = BentoBox.getInstance();
@Override
public Location getNextLocation(World world) {
Location last = plugin.getIslands().getLast(world);
if (last == null) {
@ -46,10 +49,10 @@ public class DefaultNewIslandLocationStrategy implements NewIslandLocationStrate
last = Util.getClosestIsland(last);
Result r = isIsland(last);
while (!r.equals(Result.FREE) && result.getOrDefault(Result.BLOCK_AT_CENTER, 0) < MAX_UNOWNED_ISLANDS) {
while (!r.equals(Result.FREE) && result.getOrDefault(Result.BLOCKS_IN_AREA, 0) < MAX_UNOWNED_ISLANDS) {
nextGridLocation(last);
last = Util.getClosestIsland(last);
result.merge(r, 1, (k, v) -> v++);
result.put(r, result.getOrDefault(r, 0) + 1);
r = isIsland(last);
}
@ -57,8 +60,6 @@ public class DefaultNewIslandLocationStrategy implements NewIslandLocationStrate
// We could not find a free spot within the limit required. It's likely this
// world is not empty
plugin.logError("Could not find a free spot for islands! Is this world empty?");
plugin.logError("Blocks at center locations: " + result.getOrDefault(Result.BLOCK_AT_CENTER, 0) + " max "
+ MAX_UNOWNED_ISLANDS);
plugin.logError("Blocks around center locations: " + result.getOrDefault(Result.BLOCKS_IN_AREA, 0) + " max "
+ MAX_UNOWNED_ISLANDS);
plugin.logError("Known islands: " + result.getOrDefault(Result.ISLAND_FOUND, 0) + " max unlimited.");
@ -70,49 +71,41 @@ public class DefaultNewIslandLocationStrategy implements NewIslandLocationStrate
/**
* Checks if there is an island or blocks at this location
*
*
* @param location - the location
* @return true if island found, null if blocks found, false if nothing found
*/
protected Result isIsland(Location location) {
World world = location.getWorld();
// Check 4 corners
int dist = plugin.getIWM().getIslandDistance(location.getWorld());
Set<Location> locs = new HashSet<>();
locs.add(location);
locs.add(new Location(location.getWorld(), location.getX() - dist, 0, location.getZ() - dist));
locs.add(new Location(location.getWorld(), location.getX() - dist, 0, location.getZ() + dist - 1));
locs.add(new Location(location.getWorld(), location.getX() + dist - 1, 0, location.getZ() - dist));
locs.add(new Location(location.getWorld(), location.getX() + dist - 1, 0, location.getZ() + dist - 1));
locs.add(new Location(world, location.getX() - dist, 0, location.getZ() - dist));
locs.add(new Location(world, location.getX() - dist, 0, location.getZ() + dist - 1));
locs.add(new Location(world, location.getX() + dist - 1, 0, location.getZ() - dist));
locs.add(new Location(world, location.getX() + dist - 1, 0, location.getZ() + dist - 1));
boolean generated = false;
for (Location l : locs) {
if (plugin.getIslands().getIslandAt(l).isPresent() || plugin.getIslandDeletionManager().inDeletion(l)) {
return Result.ISLAND_FOUND;
}
if (Util.isChunkGenerated(l)) generated = true;
}
if (!plugin.getIWM().isUseOwnGenerator(location.getWorld())) {
// Block check
if (!location.getBlock().isEmpty() && !location.getBlock().getType().equals(Material.WATER)) {
plugin.getIslands().createIsland(location);
return Result.BLOCK_AT_CENTER;
}
// Look around
for (int x = -5; x <= 5; x++) {
for (int y = 10; y < location.getWorld().getMaxHeight(); y++) {
for (int z = -5; z <= 5; z++) {
if (!location.getWorld().getBlockAt(x + location.getBlockX(), y, z + location.getBlockZ())
.isEmpty()
&& !location.getWorld()
.getBlockAt(x + location.getBlockX(), y, z + location.getBlockZ()).getType()
.equals(Material.WATER)) {
plugin.getIslands().createIsland(location);
return Result.BLOCKS_IN_AREA;
}
}
}
}
// If chunk has not been generated yet, then it's not occupied
if (!generated) {
return Result.FREE;
}
// Block check
if (!plugin.getIWM().isUseOwnGenerator(world) && Arrays.asList(BlockFace.values()).stream().anyMatch(bf -> location.getBlock().getRelative(bf).isEmpty()
&& !location.getBlock().getRelative(bf).getType().equals(Material.WATER))) {
// Block found
plugin.getIslands().createIsland(location);
return Result.BLOCKS_IN_AREA;
}
return Result.FREE;
}

View File

@ -35,7 +35,7 @@ public class NewIsland {
private NewIslandLocationStrategy locationStrategy;
public NewIsland(Builder builder) {
public NewIsland(Builder builder) throws Exception {
plugin = BentoBox.getInstance();
this.user = builder.user2;
this.reason = builder.reason2;
@ -135,9 +135,9 @@ public class NewIsland {
/**
* @return Island
* @throws IOException - if there are insufficient parameters defined
* @throws Exception
*/
public Island build() throws IOException {
public Island build() throws Exception {
if (user2 != null) {
NewIsland newIsland = new NewIsland(this);
return newIsland.getIsland();
@ -149,8 +149,9 @@ public class NewIsland {
/**
* Makes an island.
* @param oldIsland old island that is being replaced, if any
* @throws IOException - if an island cannot be made. Message is the tag to show the user.
*/
public void newIsland(Island oldIsland) {
public void newIsland(Island oldIsland) throws IOException {
Location next = null;
if (plugin.getIslands().hasIsland(world, user)) {
// Island exists, it just needs pasting
@ -168,14 +169,15 @@ public class NewIsland {
if (next == null) {
next = this.locationStrategy.getNextLocation(world);
if (next == null) {
plugin.logError("Failed to make island - no unoccupied spot found");
return;
plugin.logError("Failed to make island - no unoccupied spot found.");
plugin.logError("If the world was imported, try multiple times until all unowned islands are known.");
throw new IOException("commands.island.create.cannot-create-island");
}
// Add to the grid
island = plugin.getIslands().createIsland(next, user.getUniqueId());
if (island == null) {
plugin.logError("Failed to make island! Island could not be added to the grid.");
return;
throw new IOException("commands.island.create.unable-create-island");
}
}

View File

@ -431,8 +431,9 @@ commands:
description: "create an island, using optional blueprint (requires permission)"
parameters: "<blueprint>"
too-many-islands: "&c There are too many islands in this world: there isn't enough room for yours to be created."
cannot-create-island: "&c A spot could not be found in time, please try again..."
unable-create-island: "&c Your island could not be generated, please contact an administrator."
creating-island: "&a We found a spot for your island, let us prepare it for you."
creating-island: "&a Finding a spot for your island..."
pasting:
estimated-time: "&a Estimated time: &b [number] &a seconds."
blocks: "&a Building it block by block: &b [number] &a blocks in all..."

View File

@ -233,7 +233,7 @@ public class IslandCreateCommandTest {
* @throws IOException
*/
@Test
public void testExecuteUserStringListOfStringSuccess() throws IOException {
public void testExecuteUserStringListOfStringSuccess() throws Exception {
// Bundle exists
when(bpm.validate(any(), any())).thenReturn("custom");
// Has permission
@ -253,17 +253,17 @@ public class IslandCreateCommandTest {
* @throws IOException
*/
@Test
public void testExecuteUserStringListOfStringThrowException() throws IOException {
public void testExecuteUserStringListOfStringThrowException() throws Exception {
// Bundle exists
when(bpm.validate(any(), any())).thenReturn("custom");
// Has permission
when(bpm.checkPerm(any(), any(), any())).thenReturn(true);
when(builder.build()).thenThrow(new IOException("message"));
when(builder.build()).thenThrow(new IOException("commands.island.create.unable-create-island"));
assertFalse(cc.execute(user, "", Collections.singletonList("custom")));
verify(user).sendMessage("commands.island.create.creating-island");
verify(user).sendMessage("commands.island.create.unable-create-island");
verify(plugin).logError("Could not create island for player. message");
verify(plugin).logError("Could not create island for player. commands.island.create.unable-create-island");
}
/**
@ -305,7 +305,7 @@ public class IslandCreateCommandTest {
* @throws IOException
*/
@Test
public void testExecuteUserStringListOfStringKnownBundle() throws IOException {
public void testExecuteUserStringListOfStringKnownBundle() throws Exception {
// Has permission
when(bpm.checkPerm(any(), any(), any())).thenReturn(true);
when(bpm.validate(any(), any())).thenReturn("custom");

View File

@ -229,7 +229,7 @@ public class IslandResetCommandTest {
* Test method for {@link IslandResetCommand#execute(User, String, java.util.List)}
*/
@Test
public void testNoConfirmationRequired() throws IOException {
public void testNoConfirmationRequired() throws Exception {
// Now has island, but is not the owner
when(im.hasIsland(any(), eq(uuid))).thenReturn(true);
// Set so no confirmation required
@ -269,7 +269,7 @@ public class IslandResetCommandTest {
* Test method for {@link IslandResetCommand#canExecute(User, String, java.util.List)}
*/
@Test
public void testUnlimitedResets() throws IOException {
public void testUnlimitedResets() throws Exception {
// Now has island, but is not the owner
when(im.hasIsland(any(), eq(uuid))).thenReturn(true);
// Now has no team
@ -302,7 +302,7 @@ public class IslandResetCommandTest {
* Test method for {@link IslandResetCommand#canExecute(User, String, java.util.List)}
*/
@Test
public void testNoPaste() throws IOException {
public void testNoPaste() throws Exception {
irc = new IslandResetCommand(ic, true);
// Now has island, but is not the owner
when(im.hasIsland(any(), eq(uuid))).thenReturn(true);
@ -335,7 +335,7 @@ public class IslandResetCommandTest {
* Test method for {@link IslandResetCommand#execute(User, String, java.util.List)}
*/
@Test
public void testConfirmationRequired() throws IOException {
public void testConfirmationRequired() throws Exception {
// Now has island, but is not the owner
when(im.hasIsland(any(), eq(uuid))).thenReturn(true);
// Now is owner, but still has team
@ -405,7 +405,7 @@ public class IslandResetCommandTest {
* Test method for {@link IslandResetCommand#execute(User, String, java.util.List)}
*/
@Test
public void testNoConfirmationRequiredCustomSchemHasPermission() throws IOException {
public void testNoConfirmationRequiredCustomSchemHasPermission() throws Exception {
// Now has island, but is not the owner
when(im.hasIsland(any(), eq(uuid))).thenReturn(true);
// Now is owner, but still has team

View File

@ -9,7 +9,6 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.IOException;
import java.util.Collections;
import java.util.Optional;
import java.util.UUID;
@ -175,23 +174,23 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderNoUser(){
try {
NewIsland.builder().build();
} catch (IOException e) {
} catch (Exception e) {
assertEquals("Insufficient parameters. Must have a user!", e.getMessage());
}
}
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilder() throws IOException {
public void testBuilder() throws Exception {
NewIsland.builder().addon(addon).name(NAME).player(user).noPaste().reason(Reason.CREATE).oldIsland(oldIsland).build();
// Verifications
verify(im).save(eq(island));
@ -208,10 +207,10 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderReset() throws IOException {
public void testBuilderReset() throws Exception {
when(builder.build()).thenReturn(ire);
NewIsland.builder().addon(addon).name(NAME).player(user).noPaste().reason(Reason.RESET).oldIsland(oldIsland).build();
// Verifications
@ -229,10 +228,10 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderNoOldIsland() throws IOException {
public void testBuilderNoOldIsland() throws Exception {
NewIsland.builder().addon(addon).name(NAME).player(user).noPaste().reason(Reason.CREATE).build();
// Verifications
verify(im).save(eq(island));
@ -248,10 +247,10 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderNoOldIslandPaste() throws IOException {
public void testBuilderNoOldIslandPaste() throws Exception {
NewIsland.builder().addon(addon).name(NAME).player(user).reason(Reason.CREATE).build();
// Verifications
verify(im).save(eq(island));
@ -267,10 +266,10 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderHasIsland() throws IOException {
public void testBuilderHasIsland() throws Exception {
when(im.hasIsland(any(), any(User.class))).thenReturn(true);
NewIsland.builder().addon(addon).name(NAME).player(user).noPaste().reason(Reason.CREATE).oldIsland(oldIsland).build();
// Verifications
@ -289,10 +288,10 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderHasIslandFail() throws IOException {
public void testBuilderHasIslandFail() throws Exception {
when(im.getIsland(any(), any(User.class))).thenReturn(null);
when(im.hasIsland(any(), any(User.class))).thenReturn(true);
NewIsland.builder().addon(addon).name(NAME).player(user).noPaste().reason(Reason.CREATE).oldIsland(oldIsland).build();
@ -312,10 +311,10 @@ public class NewIslandTest {
/**
* Test method for {@link world.bentobox.bentobox.managers.island.NewIsland#builder()}.
* @throws IOException
* @throws Exception
*/
@Test
public void testBuilderHasIslandFailnoReserve() throws IOException {
public void testBuilderHasIslandFailnoReserve() throws Exception {
when(island.isReserved()).thenReturn(false);
when(im.hasIsland(any(), any(User.class))).thenReturn(true);
NewIsland.builder().addon(addon).name(NAME).player(user).noPaste().reason(Reason.CREATE).oldIsland(oldIsland).build();