Skip to content

Commit f6e92b3

Browse files
tastybentoclaude
andcommitted
Fix purge regions: nether detection, login check inversion, and add scan report
- isNether/isEnd were computed once at constructor time before IWM had loaded the addon world config, causing the Nether to always appear disabled. Now evaluated at the start of findIslands() each run. - canDeleteIsland() had the recent-login branch returning false ("can delete") instead of true ("cannot delete"), and used findFirst() instead of anyMatch() so only the first team member was checked. - Replace the removeIf filter with an explicit iterator loop that counts islands blocked by level threshold and purge protection, and reports how many regions each prevents from being purged. - Add an immediate "please wait" notice right after the candidate count is logged, before the multi-second freshness/island check begins. - Update the bug-documenting test to assert the now-correct behaviour: an island whose member logged in recently is excluded from purge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ff3aec5 commit f6e92b3

2 files changed

Lines changed: 87 additions & 43 deletions

File tree

src/main/java/world/bentobox/bentobox/api/commands/admin/purge/AdminPurgeRegionsCommand.java

Lines changed: 83 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ public class AdminPurgeRegionsCommand extends CompositeCommand implements Listen
5050
private boolean toBeConfirmed;
5151
private User user;
5252
private Map<Pair<Integer, Integer>, Set<String>> deleteableRegions;
53-
private final boolean isNether;
54-
private final boolean isEnd;
53+
private boolean isNether;
54+
private boolean isEnd;
5555
private int days;
5656

5757
public AdminPurgeRegionsCommand(CompositeCommand parent) {
5858
super(parent, "regions");
5959
getAddon().registerListener(this);
60-
isNether = getPlugin().getIWM().isNetherGenerate(getWorld()) && getPlugin().getIWM().isNetherIslands(getWorld());
61-
isEnd = getPlugin().getIWM().isEndGenerate(getWorld()) && getPlugin().getIWM().isEndIslands(getWorld());
60+
// isNether/isEnd are NOT computed here: IWM may not have loaded the addon world
61+
// config yet at command-registration time. They are evaluated lazily in findIslands().
6262
}
6363

6464
@Override
@@ -200,7 +200,7 @@ private void deletePlayerFile(File file, String description) {
200200
* subfolder that Minecraft uses for non-overworld environments.
201201
* <p>
202202
* Overworld data lives directly in the world folder, but Nether data lives
203-
* in {@code DIM-1/} and End data lives in {@code DIM1/} subfolders even
203+
* in {@code DIM-1/} and End data lives in {@code DIM1/} subfolders - even
204204
* when the world has its own separate folder.
205205
*
206206
* @param world the world to resolve
@@ -327,11 +327,14 @@ private boolean deleteOneRegion(String name, DimFolders overworld, DimFolders ne
327327
}
328328

329329
/**
330-
* This method is run async!
330+
* This method is run async!
331331
* @param world world
332332
* @param days days old
333333
*/
334334
private void findIslands(World world, int days) {
335+
// Evaluate here, not in the constructor - IWM config is loaded by the time a command runs
336+
isNether = getPlugin().getIWM().isNetherGenerate(world) && getPlugin().getIWM().isNetherIslands(world);
337+
isEnd = getPlugin().getIWM().isEndGenerate(world) && getPlugin().getIWM().isEndIslands(world);
335338
try {
336339
// Get the grid that covers this world
337340
IslandGrid islandGrid = getPlugin().getIslands().getIslandCache().getIslandGrid(world);
@@ -343,17 +346,55 @@ private void findIslands(World world, int days) {
343346
List<Pair<Integer, Integer>> oldRegions = this.findOldRegions(days);
344347
// Get islands that are associated with these regions
345348
deleteableRegions = this.mapIslandsToRegions(oldRegions, islandGrid);
346-
// Remove any region whose island‐set contains at least one island that either isn’t found or fails the deletion check:
347-
deleteableRegions.values().removeIf(islandIds ->
348-
islandIds.stream()
349-
.map(getPlugin().getIslands()::getIslandById) // Stream<Optional<Island>>
350-
.anyMatch(optIsland ->
351-
// If missing (empty) → treat as undeletable (true) - this is a bit conservative but maybe the database is messed up
352-
// If present, checkIsland(...) == true means “cannot delete”
353-
optIsland.map(this::canDeleteIsland)
354-
.orElse(true)
355-
)
356-
);
349+
// Filter regions: remove any whose island-set contains at least one island that cannot be deleted.
350+
// Track why islands are blocked so we can show a summary report.
351+
int islandsOverLevel = 0;
352+
int islandsPurgeProtected = 0;
353+
int regionsBlockedByLevel = 0;
354+
int regionsBlockedByProtection = 0;
355+
356+
var iter = deleteableRegions.entrySet().iterator();
357+
while (iter.hasNext()) {
358+
var entry = iter.next();
359+
boolean remove = false;
360+
boolean regionHasLevelBlock = false;
361+
boolean regionHasPurgeBlock = false;
362+
for (String id : entry.getValue()) {
363+
Optional<Island> opt = getPlugin().getIslands().getIslandById(id);
364+
if (opt.isEmpty()) {
365+
remove = true;
366+
continue;
367+
}
368+
Island isl = opt.get();
369+
if (canDeleteIsland(isl)) {
370+
remove = true;
371+
if (isl.isPurgeProtected()) {
372+
islandsPurgeProtected++;
373+
regionHasPurgeBlock = true;
374+
}
375+
if (isLevelTooHigh(isl)) {
376+
islandsOverLevel++;
377+
regionHasLevelBlock = true;
378+
}
379+
}
380+
}
381+
if (remove) {
382+
iter.remove();
383+
if (regionHasLevelBlock) regionsBlockedByLevel++;
384+
if (regionHasPurgeBlock) regionsBlockedByProtection++;
385+
}
386+
}
387+
388+
// Summary report
389+
if (islandsOverLevel > 0) {
390+
getPlugin().log("Purge: " + islandsOverLevel + " island(s) exceed the level threshold of "
391+
+ getPlugin().getSettings().getIslandPurgeLevel()
392+
+ " - preventing " + regionsBlockedByLevel + " region(s) from being purged");
393+
}
394+
if (islandsPurgeProtected > 0) {
395+
getPlugin().log("Purge: " + islandsPurgeProtected + " island(s) are purge-protected"
396+
+ " - preventing " + regionsBlockedByProtection + " region(s) from being purged");
397+
}
357398
// At this point any islands that might be deleted are in the cache, and so we can freely access them
358399
// 1) Pull out all island IDs,
359400
// 2) resolve to Optional<Island>,
@@ -429,35 +470,45 @@ private String formatLocalTimestamp(Long millis) {
429470
}
430471

431472
/**
432-
* Check if an island can be deleted or not. Purge protected, spawn, or unowned islands cannot be deleted.
433-
* Islands over a certain level cannot be deleted.
473+
* Check if an island cannot be deleted. Purge protected, spawn, or unowned islands cannot be deleted.
474+
* Islands whose members recently logged in, or that exceed the level threshold, cannot be deleted.
434475
* @param island island
435-
* @return true means cannot delete
476+
* @return true means "cannot delete"
436477
*/
437478
private boolean canDeleteIsland(Island island) {
438-
// If the island is deletable, it can be deleted at any time
479+
// If the island is marked deletable it can always be purged
439480
if (island.isDeletable()) {
440481
return false;
441482
}
442483
long cutoffMillis = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(days);
443-
// Check if owner or team members logged in recently
444-
if (island.getMemberSet().stream().map(uuid -> {
484+
// Block if ANY member (owner or team) has logged in within the cutoff window
485+
boolean recentLogin = island.getMemberSet().stream().anyMatch(uuid -> {
445486
Long lastLogin = getPlugin().getPlayers().getLastLoginTimestamp(uuid);
446487
if (lastLogin == null) {
447488
lastLogin = Bukkit.getOfflinePlayer(uuid).getLastSeen();
448489
}
449490
return lastLogin >= cutoffMillis;
450-
}).findFirst().orElse(false)) {
451-
return false;
491+
});
492+
if (recentLogin) {
493+
return true;
452494
}
453-
// Level check
454-
boolean levelCheck = getPlugin().getAddonsManager().getAddonByName("Level").map(l ->
455-
((Level) l).getIslandLevel(getWorld(), island.getOwner()) >= getPlugin().getSettings().getIslandPurgeLevel()).orElse(false);
456-
if (levelCheck) {
457-
// Island level is too high
495+
if (isLevelTooHigh(island)) {
458496
return true;
459497
}
460-
return island.isPurgeProtected() || island.isSpawn() || !island.isOwned();
498+
return island.isPurgeProtected() || island.isSpawn() || !island.isOwned();
499+
}
500+
501+
/**
502+
* Returns true if the island's level meets or exceeds the configured purge threshold.
503+
* Returns false when the Level addon is not present.
504+
* @param island island to check
505+
* @return true if the island level is too high to purge
506+
*/
507+
private boolean isLevelTooHigh(Island island) {
508+
return getPlugin().getAddonsManager().getAddonByName("Level")
509+
.map(l -> ((Level) l).getIslandLevel(getWorld(), island.getOwner())
510+
>= getPlugin().getSettings().getIslandPurgeLevel())
511+
.orElse(false);
461512
}
462513

463514
/**
@@ -498,6 +549,7 @@ private List<Pair<Integer, Integer>> findOldRegions(int days) {
498549
// file was already deleted by a previous (buggy) purge run.
499550
Set<String> candidateNames = collectCandidateNames(overworldRegion, netherRegion, endRegion);
500551
getPlugin().log("Purge total candidate region coordinates: " + candidateNames.size());
552+
getPlugin().log("Purge checking candidate region(s) against island data, please wait...");
501553

502554
List<Pair<Integer, Integer>> regions = new ArrayList<>();
503555
for (String name : candidateNames) {

src/test/java/world/bentobox/bentobox/api/commands/admin/purge/AdminPurgeRegionsCommandTest.java

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -336,15 +336,10 @@ public void testExecuteRecentRegionFileExcluded() throws IOException {
336336
}
337337

338338
/**
339-
* Note: {@code canDeleteIsland()} contains an inverted login-check — when a member's last
340-
* login is &gt;= the cutoff (i.e. they logged in recently), the method returns {@code false}
341-
* ("can delete") instead of {@code true} ("cannot delete"). This test documents the actual
342-
* behaviour so the bug is visible and guarded against accidental change.
343-
*
344-
* Correct behaviour would be: island whose member logged in recently → excluded from purge.
339+
* An island whose member logged in recently must be excluded from purge candidates.
345340
*/
346341
@Test
347-
public void testExecuteIslandWithRecentLoginIsIncludedDueToBug() throws IOException {
342+
public void testExecuteIslandWithRecentLoginIsExcluded() throws IOException {
348343
UUID ownerUUID = UUID.randomUUID();
349344
when(island.getUniqueId()).thenReturn("island-1");
350345
when(island.getOwner()).thenReturn(ownerUUID);
@@ -363,17 +358,14 @@ public void testExecuteIslandWithRecentLoginIsIncludedDueToBug() throws IOExcept
363358
when(islandCache.getIslandGrid(world)).thenReturn(grid);
364359
when(im.getIslandById("island-1")).thenReturn(Optional.of(island));
365360

366-
// Owner logged in recently — should protect the island but due to the inverted
367-
// login check in canDeleteIsland() it currently does not.
361+
// Owner logged in recently — island must be protected from purge
368362
when(pm.getLastLoginTimestamp(ownerUUID)).thenReturn(System.currentTimeMillis());
369363

370364
Path regionDir = Files.createDirectories(tempDir.resolve("region"));
371365
Files.createFile(regionDir.resolve("r.0.0.mca"));
372366

373367
assertTrue(aprc.execute(user, "regions", List.of("10")));
374-
// BUG: island IS included despite recent login
375-
verify(user).sendMessage("commands.admin.purge.purgable-islands", TextVariables.NUMBER, "1");
376-
verify(user).sendMessage("commands.admin.purge.regions.confirm", TextVariables.LABEL, "regions");
368+
verify(user).sendMessage("commands.admin.purge.none-found");
377369
}
378370

379371
/**

0 commit comments

Comments
 (0)