Skip to content

Commit 27e1833

Browse files
tastybentoclaude
andcommitted
Fix inventory lost when returning to original island after creating second island
When a player transitions from single-island to multi-island mode, their currentKey is a world-only key (e.g., "oneblock_world"). Previously, when entering a new island, storeInventory saved to this world-only key, and the backward-compat migration in getInventory incorrectly gave that data to the new island instead of preserving it for the original island. Add Store.upgradeWorldKeyToIsland() which proactively upgrades the currentKey to an island-specific key before any save/load occurs. Called from PlayerListener.onIslandEnter when a world-only key is detected, this ensures storeInventory saves to the correct island-specific key and prevents the migration from misassigning data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3e03f7e commit 27e1833

4 files changed

Lines changed: 159 additions & 7 deletions

File tree

CLAUDE.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## Project Overview
6+
7+
InvSwitcher is a BentoBox addon for Minecraft (Spigot/Bukkit) that gives players separate inventories, ender chests, health, food, experience, advancements, game modes, and statistics per game-world. Nether and End dimensions are automatically grouped with their overworld.
8+
9+
## Build Commands
10+
11+
- **Build**: `mvn clean package`
12+
- **Run tests**: `mvn test -Dmaven.compiler.forceJavacCompilerUse=true`
13+
- **Run a single test class**: `mvn test -Dmaven.compiler.forceJavacCompilerUse=true -Dtest=StoreTest`
14+
- **Run a single test method**: `mvn test -Dmaven.compiler.forceJavacCompilerUse=true -Dtest=StoreTest#testMethodName`
15+
16+
Requires Java 21. The build produces a shaded JAR in `target/`. The `-Dmaven.compiler.forceJavacCompilerUse=true` flag is needed to work around a compiler hashing bug with the current JDK.
17+
18+
## Architecture
19+
20+
This is a BentoBox Addon (extends `Addon`, not a standalone Bukkit plugin). Key flow:
21+
22+
- **InvSwitcher** - Addon entry point. Loads config, resolves configured world names to `World` objects (including nether/end variants), creates the `Store`, and registers `PlayerListener`.
23+
- **Store** - Core logic. Maintains an in-memory `Map<UUID, InventoryStorage>` cache backed by BentoBox's `Database`. On world change: stores current player state to the old world's slot, clears the player, then loads the new world's slot. Handles XP math manually (Bukkit's `getTotalExperience()` is unreliable). Tracks per-player `currentKey` to map saves/loads to the correct storage slot.
24+
- **InventoryStorage** - `DataObject` (BentoBox DB entity) keyed by player UUID. All per-world data is stored as `Map<String, ...>` where the key is either the overworld name (e.g., `"oneblock_world"`) or an island-specific key (e.g., `"oneblock_world/islandId"`). Persisted via BentoBox's database abstraction (JSON, MySQL, etc. — **not** YAML, which is explicitly unsupported).
25+
- **PlayerListener** - Listens to `PlayerChangedWorldEvent`, `PlayerJoinEvent`, `PlayerQuitEvent`, `IslandEnterEvent`, and `PlayerRespawnEvent` to trigger store/load operations.
26+
- **Settings** - `ConfigObject` loaded from `config.yml`. Each switchable aspect (inventory, health, food, etc.) has a world-level boolean toggle and a per-island sub-toggle.
27+
28+
## Key Design Details
29+
30+
- World name normalization: nether (`_nether`) and end (`_the_end`) suffixes are stripped to map all three dimensions to the same overworld key.
31+
- **Per-island inventory switching**: When `islandsActive` is enabled and a player owns multiple concurrent islands, data is keyed as `"worldName/islandId"` instead of just `"worldName"`. Each data type (inventory, health, etc.) has its own per-island sub-toggle.
32+
- **Storage key transitions**: When a player goes from 1 island to multiple, their `currentKey` must be upgraded from a world-only key to an island-specific key. This is handled proactively in `PlayerListener.onIslandEnter` via `Store.upgradeWorldKeyToIsland()` before any save/load occurs.
33+
- **Backward compatibility migration**: `Store.getInventory()` migrates world-only data to island-specific keys on first load if no island-specific data exists yet.
34+
- Statistics saving runs asynchronously via `Bukkit.getScheduler()` except during shutdown (where scheduling is unavailable).
35+
- Tests use JUnit 5 + MockBukkit + Mockito. The surefire plugin requires extensive `--add-opens` flags (already configured in pom.xml).

src/main/java/com/wasteofplastic/invswitcher/Store.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,23 @@ public void removeFromCache(Player player) {
305305
currentKey.remove(player.getUniqueId());
306306
}
307307

308+
/**
309+
* Upgrade a world-only currentKey to an island-specific key when a player transitions
310+
* from single-island to multi-island mode. Clears stale world-only data from the store
311+
* (storeAndSave will re-save world-level types like health/food to the world key).
312+
* @param player - player
313+
* @param world - world
314+
* @param oldIsland - the island the player was on (their original island)
315+
*/
316+
public void upgradeWorldKeyToIsland(Player player, World world, Island oldIsland) {
317+
InventoryStorage store = getInv(player);
318+
String worldKey = getOverworldName(world);
319+
// Clear stale world-only data; storeAndSave will re-save world-level types
320+
store.clearWorldData(worldKey);
321+
// Update currentKey so subsequent storeAndSave saves per-island data to the correct key
322+
currentKey.put(player.getUniqueId(), worldKey + "/" + oldIsland.getUniqueId());
323+
}
324+
308325
/**
309326
* Get the inventory storage object for player from the database or make a new one
310327
* @param player - player

src/main/java/com/wasteofplastic/invswitcher/listeners/PlayerListener.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.wasteofplastic.invswitcher.InvSwitcher;
1818

19-
import world.bentobox.bentobox.BentoBox;
2019
import world.bentobox.bentobox.api.events.island.IslandEnterEvent;
2120
import world.bentobox.bentobox.database.objects.Island;
2221
import world.bentobox.bentobox.util.Util;
@@ -68,7 +67,6 @@ public void onWorldEnter(final PlayerChangedWorldEvent event) {
6867
*/
6968
@EventHandler(priority = EventPriority.HIGH, ignoreCancelled = true)
7069
public void onIslandEnter(IslandEnterEvent event) {
71-
BentoBox.getInstance().logDebug("IslandEnterEvent triggered for player " + event.getPlayerUUID() + " on island " + event.getIsland().getUniqueId());
7270
if (!addon.getSettings().isIslandsActive()) {
7371
return;
7472
}
@@ -80,15 +78,13 @@ public void onIslandEnter(IslandEnterEvent event) {
8078

8179
World world = player.getWorld();
8280
if (!addon.getWorlds().contains(world)) {
83-
BentoBox.getInstance().logDebug("World " + world.getName() + " is not in the list of worlds to manage. Ignoring island enter event.");
8481
return;
8582
}
8683

8784
Island island = event.getIsland();
8885

8986
// Only switch if the player owns the island they're entering
9087
if (island.getOwner() == null || !island.getOwner().equals(player.getUniqueId())) {
91-
BentoBox.getInstance().logDebug("Player " + player.getName() + " does not own island " + island.getUniqueId() + ". No inventory switch.");
9288
return;
9389
}
9490

@@ -97,20 +93,33 @@ public void onIslandEnter(IslandEnterEvent event) {
9793
int count = addon.getIslands().getNumberOfConcurrentIslands(
9894
player.getUniqueId(), Objects.requireNonNull(overworld));
9995
if (count <= 1) {
100-
BentoBox.getInstance().logDebug("Player " + player.getName() + " owns only one island in world " + overworld.getName() + ". No inventory switch.");
10196
return;
10297
}
10398

10499
// Compute new key and compare to current key
105100
String newKey = addon.getStore().getStorageKey(player, world, island);
106101
String currentKeyValue = addon.getStore().getCurrentKey(player);
107102
if (newKey.equals(currentKeyValue)) {
108-
BentoBox.getInstance().logDebug("Player " + player.getName() + " is entering island " + island.getUniqueId() + " which has the same inventory key as their current location. No inventory switch.");
109103
return; // same island, no switch
110104
}
111105

106+
// If currentKey is a world-only key (no "/"), the player is transitioning from
107+
// single-island to multi-island mode. Upgrade the key so storeInventory saves to
108+
// the correct island-specific key instead of the world key.
109+
if (currentKeyValue != null && !currentKeyValue.contains("/")) {
110+
Island oldIsland = addon.getIslands().getIsland(overworld, player.getUniqueId());
111+
if (oldIsland != null && !oldIsland.getUniqueId().equals(island.getUniqueId())) {
112+
addon.getStore().upgradeWorldKeyToIsland(player, world, oldIsland);
113+
} else {
114+
// Primary island is the one being entered; find another owned island
115+
addon.getIslands().getIslands(overworld, player.getUniqueId()).stream()
116+
.filter(i -> !i.getUniqueId().equals(island.getUniqueId()))
117+
.findFirst()
118+
.ifPresent(i -> addon.getStore().upgradeWorldKeyToIsland(player, world, i));
119+
}
120+
}
121+
112122
// Switch: store old, load new
113-
BentoBox.getInstance().logDebug("Switching inventory for player " + player.getName() + " from key " + currentKeyValue + " to new key " + newKey);
114123
addon.getStore().storeInventory(player, world);
115124
addon.getStore().getInventory(player, world, island);
116125
}

src/test/java/com/wasteofplastic/invswitcher/StoreTest.java

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,4 +421,95 @@ public void testRemoveFromCacheClearsCurrentKey() {
421421
assertNull(s.getCurrentKey(player));
422422
}
423423

424+
/**
425+
* Test upgradeWorldKeyToIsland clears world data and updates currentKey.
426+
*/
427+
@Test
428+
public void testUpgradeWorldKeyToIsland() {
429+
sets.setIslandsActive(true);
430+
sets.setStatistics(false);
431+
432+
Island oldIsland = mock(Island.class);
433+
when(oldIsland.getOwner()).thenReturn(playerUUID);
434+
when(oldIsland.getUniqueId()).thenReturn("island-primary");
435+
436+
try (MockedStatic<Util> utilities = Mockito.mockStatic(Util.class);
437+
MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
438+
utilities.when(() -> Util.getWorld(world)).thenReturn(world);
439+
when(islandsManager.getNumberOfConcurrentIslands(playerUUID, world)).thenReturn(1);
440+
441+
// Simulate login and play — data saved under world-only key
442+
s.getInventory(player, world);
443+
assertEquals("world", s.getCurrentKey(player));
444+
s.storeInventory(player, world);
445+
assertTrue(s.isWorldStored(player, world));
446+
447+
// Upgrade: transitions from world-only to island-specific key
448+
s.upgradeWorldKeyToIsland(player, world, oldIsland);
449+
450+
// currentKey should now be island-specific
451+
assertEquals("world/island-primary", s.getCurrentKey(player));
452+
// World-only data should be cleared
453+
assertFalse(s.isWorldStored(player, world));
454+
}
455+
}
456+
457+
/**
458+
* Full scenario: player has 1 island, creates 2nd, goes to new island, returns to original.
459+
* Simulates what onIslandEnter does: upgradeWorldKey, storeInventory, getInventory.
460+
*/
461+
@Test
462+
public void testFullScenarioSingleToMultipleIslands() {
463+
sets.setIslandsActive(true);
464+
sets.setStatistics(false);
465+
sets.setHealth(false);
466+
sets.setFood(false);
467+
sets.setExperience(false);
468+
sets.setGamemode(false);
469+
sets.setAdvancements(false);
470+
sets.setEnderChest(false);
471+
472+
Island primaryIsland = mock(Island.class);
473+
when(primaryIsland.getOwner()).thenReturn(playerUUID);
474+
when(primaryIsland.getUniqueId()).thenReturn("island-primary");
475+
476+
Island newIsland = mock(Island.class);
477+
when(newIsland.getOwner()).thenReturn(playerUUID);
478+
when(newIsland.getUniqueId()).thenReturn("island-new");
479+
480+
Location loc = mock(Location.class);
481+
when(player.getLocation()).thenReturn(loc);
482+
483+
try (MockedStatic<Util> utilities = Mockito.mockStatic(Util.class);
484+
MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
485+
utilities.when(() -> Util.getWorld(world)).thenReturn(world);
486+
when(islandsManager.getNumberOfConcurrentIslands(playerUUID, world)).thenReturn(1);
487+
488+
// Step 1: Player has 1 island. Login and play.
489+
s.getInventory(player, world);
490+
s.storeInventory(player, world);
491+
assertTrue(s.isWorldStored(player, world));
492+
493+
// Step 2: Player creates 2nd island and teleports to it.
494+
// onIslandEnter detects world-only key and upgrades BEFORE store/load.
495+
when(islandsManager.getNumberOfConcurrentIslands(playerUUID, world)).thenReturn(2);
496+
s.upgradeWorldKeyToIsland(player, world, primaryIsland);
497+
assertEquals("world/island-primary", s.getCurrentKey(player));
498+
499+
// Now storeInventory saves to the island-specific key for the OLD island
500+
s.storeInventory(player, world);
501+
// Load new island — no world data to migrate, so player gets empty inventory
502+
s.getInventory(player, world, newIsland);
503+
assertEquals("world/island-new", s.getCurrentKey(player));
504+
505+
// Step 3: Player teleports back to primary island.
506+
s.storeInventory(player, world);
507+
s.getInventory(player, world, primaryIsland);
508+
assertEquals("world/island-primary", s.getCurrentKey(player));
509+
510+
// Verify inventory was loaded (setContents called for the primary island load)
511+
verify(player.getInventory(), atLeastOnce()).setContents(any(ItemStack[].class));
512+
}
513+
}
514+
424515
}

0 commit comments

Comments
 (0)