Skip to content

Better grid storage for arbitrary island positions. #2760

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
tastybento merged 3 commits into from
Nov 1, 2025
+325 −17
Merged

Better grid storage for arbitrary island positions. #2760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
662c783
Better grid storage for arbitrary island positions.
tastybento Oct 31, 2025
c055a23
Add more test cases
tastybento Oct 31, 2025
32061e8
Replace tabComplete code that was accidentally removed
tastybento Oct 31, 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
8 changes: 8 additions & 0 deletions src/main/java/world/bentobox/bentobox/api/commands/island/IslandGoCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ private boolean checkReserved(User user, List<Island> islands) {
}
return false;
}

@Override
public Optional<List<String>> tabComplete(User user, String alias, List<String> args) {
String lastArg = !args.isEmpty() ? args.get(args.size()-1) : "";

return Optional.of(Util.tabLimit(new ArrayList<>(getNameIslandMap(user, getWorld()).keySet()), lastArg));

}

/**
* Record to store island information and whether the name refers to
Expand Down
77 changes: 60 additions & 17 deletions src/main/java/world/bentobox/bentobox/managers/island/IslandGrid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,73 @@ public IslandGrid(IslandCache im) {
* @return true if successfully added, false if island already exists, or there is an overlap
*/
public boolean addToGrid(Island island) {
// Check if we know about this island already
int minX = island.getMinX();
int minZ = island.getMinZ();
IslandData islandData = new IslandData(island.getUniqueId(), minX, minZ, island.getRange());
if (grid.containsKey(minX)) {
TreeMap<Integer, IslandData> zEntry = grid.get(minX);
if (zEntry.containsKey(minZ)) {
// If it is the same island then it's okay
return island.getUniqueId().equals(zEntry.get(minZ).id());
// Island overlap, report error
} else {
// Add island
zEntry.put(minZ, islandData);
grid.put(minX, zEntry);
int range = island.getRange();
IslandData newIsland = new IslandData(island.getUniqueId(), minX, minZ, range);

// Remove this island if it is already in the grid
this.removeFromGrid(island);

// compute bounds for the new island (upper bounds are exclusive)
int newMaxX = minX + range * 2;
int newMaxZ = minZ + range * 2;

/*
* Find any existing islands that could overlap:
* - Any existing island with minX <= newMaxX could extend over newMinX, so we must consider
* all entries with key <= newMaxX (use headMap).
* - For each candidate X entry, consider Z entries with minZ <= newMaxZ (use headMap).
* This avoids missing large islands whose minX is far left of the new island.
*/
for (Entry<Integer, TreeMap<Integer, IslandData>> xEntry : grid.headMap(newMaxX, true).entrySet()) {
TreeMap<Integer, IslandData> zMap = xEntry.getValue();
for (Entry<Integer, IslandData> zEntry : zMap.headMap(newMaxZ, true).entrySet()) {
IslandData existingIsland = zEntry.getValue();
if (isOverlapping(newIsland, existingIsland)) {
return false;
}
}
} else {
// Add island
TreeMap<Integer, IslandData> zEntry = new TreeMap<>();
zEntry.put(minZ, islandData);
grid.put(minX, zEntry);
}

// No overlaps found, add the island
addNewEntry(minX, minZ, newIsland);
return true;
}

/**
* Checks if two islands overlap
* @param island1 first island
* @param island2 second island
* @return true if islands overlap
*/
private boolean isOverlapping(IslandData island1, IslandData island2) {
int island1MaxX = island1.minX() + (island1.range() * 2);
int island1MaxZ = island1.minZ() + (island1.range() * 2);
int island2MaxX = island2.minX() + (island2.range() * 2);
int island2MaxZ = island2.minZ() + (island2.range() * 2);

// Check if one rectangle is to the left of the other
if (island1MaxX <= island2.minX() || island2MaxX <= island1.minX()) {
return false;
}

// Check if one rectangle is above the other
if (island1MaxZ <= island2.minZ() || island2MaxZ <= island1.minZ()) {
return false;
}

return true;
}

/**
* Helper method to add a new entry to the grid
*/
private void addNewEntry(int minX, int minZ, IslandData islandData) {
TreeMap<Integer, IslandData> zEntry = grid.computeIfAbsent(minX, k -> new TreeMap<>());
zEntry.put(minZ, islandData);
}

/**
* Remove island from grid
* @param island - the island to remove
Expand Down
Empty file added src/test/java/world/bentobox/bentobox/managers/island/IslandGridEdgeCaseTest.java
View file
Open in desktop
Empty file.
257 changes: 257 additions & 0 deletions src/test/java/world/bentobox/bentobox/managers/island/IslandGridTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
package world.bentobox.bentobox.managers.island;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.when;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.powermock.modules.junit4.PowerMockRunner;

import world.bentobox.bentobox.database.objects.Island;

/**
* Grid test
*/
@RunWith(PowerMockRunner.class)
public class IslandGridTest {

private IslandGrid ig;
@Mock
private IslandCache im;
@Mock
private Island island;
@Mock
private Island island2;
@Mock
private Island overlappingIsland;
@Mock
private Island original;
@Mock
private Island updated;
@Mock
private Island a;
@Mock
private Island b;
@Mock
private Island big;
@Mock
private Island small;
@Mock
private Island zIsland;

/**
* @throws java.lang.Exception
*/
@Before
public void setUp() throws Exception {
// Islands
when(island.getMinX()).thenReturn(356);
when(island.getMinZ()).thenReturn(5678);
when(island.getRange()).thenReturn(64);
when(island.getUniqueId()).thenReturn("island");
when(overlappingIsland.getMinX()).thenReturn(360);
when(overlappingIsland.getMinZ()).thenReturn(5678);
when(overlappingIsland.getRange()).thenReturn(64);
when(overlappingIsland.getUniqueId()).thenReturn("overlappingIsland");
when(island2.getMinX()).thenReturn(-32);
when(island2.getMinZ()).thenReturn(-32);
when(island2.getRange()).thenReturn(64);
when(island2.getUniqueId()).thenReturn("island2");
when(im.getIslandById("island")).thenReturn(island);
when(im.getIslandById("island2")).thenReturn(island2);
when(im.getIslandById("overlappingIsland")).thenReturn(overlappingIsland);
ig = new IslandGrid(im);
}

/**
* @throws java.lang.Exception
*/
@After
public void tearDown() throws Exception {
}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#addToGrid(world.bentobox.bentobox.database.objects.Island)}.
*/
@Test
public void testAddToGrid() {
assertTrue(ig.addToGrid(island));
assertFalse(ig.addToGrid(overlappingIsland));
assertTrue(ig.addToGrid(island2));
}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#removeFromGrid(world.bentobox.bentobox.database.objects.Island)}.
*/
@Test
public void testRemoveFromGrid() {
assertTrue(ig.addToGrid(island));
assertTrue(ig.removeFromGrid(island));
assertFalse(ig.removeFromGrid(island2));
}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#getIslandAt(int, int)}.
*/
@Test
public void testGetIslandAt() {
assertNull(ig.getIslandAt(0, 0));
assertTrue(ig.addToGrid(island));
assertTrue(ig.addToGrid(island2));
assertEquals(island, ig.getIslandAt(360, 5700));
assertEquals(island2, ig.getIslandAt(0, 0));
}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#isIslandAt(int, int)}.
*/
@Test
public void testIsIslandAt() {
assertFalse(ig.isIslandAt(0, 0));
assertTrue(ig.addToGrid(island2));
assertTrue(ig.isIslandAt(0, 0));
assertTrue(ig.addToGrid(island));
assertTrue(ig.isIslandAt(360, 5700));
assertFalse(ig.isIslandAt(-1000, 1000));
}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#getIslandStringAt(int, int)}.
*/
@Test
public void testGetIslandStringAt() {
assertNull(ig.getIslandStringAt(0, 0));
assertTrue(ig.addToGrid(island2));
assertEquals("island2", ig.getIslandStringAt(0, 0));

}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#getSize()}.
*/
@Test
public void testGetSize() {
assertEquals(0, ig.getSize());
assertTrue(ig.addToGrid(island2));
assertEquals(1, ig.getSize());
assertTrue(ig.addToGrid(island));
assertEquals(2, ig.getSize());
ig.removeFromGrid(island);
assertEquals(1, ig.getSize());
ig.removeFromGrid(island2);
assertEquals(0, ig.getSize());
}

/**
* Test method for {@link world.bentobox.bentobox.managers.island.IslandGrid#getGrid()}.
*/
@Test
public void testGetGrid() {
assertNotNull(ig.getGrid());
}

@Test
public void testUpdateIslandCoordinatesKeepsSingleEntry() {
// original at (100,100) range 20
when(original.getMinX()).thenReturn(100);
when(original.getMinZ()).thenReturn(100);
when(original.getRange()).thenReturn(20);
when(original.getUniqueId()).thenReturn("orig");

// updated has same id but moved to (300,300) range 20
when(updated.getMinX()).thenReturn(300);
when(updated.getMinZ()).thenReturn(300);
when(updated.getRange()).thenReturn(20);
when(updated.getUniqueId()).thenReturn("orig");

when(im.getIslandById("orig")).thenReturn(original);

// add original
assertTrue(ig.addToGrid(original));
assertEquals(1, ig.getSize());

// add updated (same id) -> should update, keep size 1
assertTrue(ig.addToGrid(updated));
assertEquals(1, ig.getSize());

// original location should no longer contain the island
assertNull(ig.getIslandStringAt(110, 110));

// new location should contain the island
assertEquals("orig", ig.getIslandStringAt(310, 310));
}

@Test
public void testAdjacentIslandsAllowedWhenEdgesTouch() {
// island a covers x:[0,20) z:[0,20)
when(a.getMinX()).thenReturn(0);
when(a.getMinZ()).thenReturn(0);
when(a.getRange()).thenReturn(10);
when(a.getUniqueId()).thenReturn("a");

// island b starts exactly at x=20 (touching edge), z same
when(b.getMinX()).thenReturn(20);
when(b.getMinZ()).thenReturn(0);
when(b.getRange()).thenReturn(10);
when(b.getUniqueId()).thenReturn("b");

when(im.getIslandById("a")).thenReturn(a);
when(im.getIslandById("b")).thenReturn(b);

assertTrue(ig.addToGrid(a));
// touching edge should be allowed
assertTrue(ig.addToGrid(b));

// verify both retrievable at representative coords
assertEquals("a", ig.getIslandStringAt(10, 10));
assertEquals("b", ig.getIslandStringAt(21, 10));
}

@Test
public void testLargeExistingIslandShouldBlockSmallIslandEvenIfMinXOutsideSubMapWindow() {
// big island minX = 0, range = 1000
when(big.getMinX()).thenReturn(0);
when(big.getMinZ()).thenReturn(0);
when(big.getRange()).thenReturn(1000);
when(big.getUniqueId()).thenReturn("big");

// small island minX = 1500, range = 10 -> would overlap big
when(small.getMinX()).thenReturn(1500);
when(small.getMinZ()).thenReturn(10);
when(small.getRange()).thenReturn(10);
when(small.getUniqueId()).thenReturn("small");

when(im.getIslandById("big")).thenReturn(big);
when(im.getIslandById("small")).thenReturn(small);

assertTrue(ig.addToGrid(big));

// Expected: adding small should be rejected because it lies inside big
// If this test fails, it reveals the current subMap window is too small to find big.
assertFalse("Small island overlaps big island; should have been rejected", ig.addToGrid(small));
}

@Test
public void testGetIslandStringAtWhenXEntryExistsButNoZEntryApplies() {
// island exists at minX=100 minZ=100 range=10 (covers z [110,110))
when(zIsland.getMinX()).thenReturn(100);
when(zIsland.getMinZ()).thenReturn(100);
when(zIsland.getRange()).thenReturn(10);
when(zIsland.getUniqueId()).thenReturn("z");

when(im.getIslandById("z")).thenReturn(zIsland);

assertTrue(ig.addToGrid(zIsland));

// Query an x within island x-range but z is below any minZ -> should return null
assertNull(ig.getIslandStringAt(110, 50));
}

}