Release 1.19.2

.github/workflows/publish.yml

@@ -18,7 +18,7 @@ on:
 
 jobs:
   publish:
-    uses: bentoboxworld/.github/.github/workflows/publish-platforms.yml@fe4b1f03c19f4fd4212020a06a07a7097923adec # master
+    uses: bentoboxworld/.github/.github/workflows/publish-platforms.yml@1f91a0edf72e8c86d671b3b8fdd3121ac6fb88e1 # master
     with:
       use_release_asset: "true"          # publish the jar attached to the release; do not rebuild
       hangar_slug: "InvSwitcher"           # blank = skip Hangar

README.md

@@ -18,6 +18,55 @@ The following are switched per-world:
 3. Done!
 4. (Optional) If you would prefer to have achievements not broadcasted in your server chat when players change worlds, run the command `/gamerule announceAdvancements false` in-game, or in your server console by removing the "/" symbol. 
 
+## Do not run two inventory managers
+
+InvSwitcher must be the only plugin managing per-world inventories. Running it alongside
+Multiverse-Inventories, PerWorldInventory, MultiInv or similar makes both plugins save and restore
+the player on every world change, and they overwrite each other's data.
+
+The symptom is disappearing items: pick up an item on your island, go to the lobby, come back, and
+the island inventory is empty. Nothing appears in the console, because neither plugin is failing —
+each is faithfully saving a player state the other has already rewritten. The InvSwitcher version
+makes no difference, so if you are seeing this, look for a second inventory plugin first.
+
+### Multiverse-Inventories
+
+Two things commonly mislead admins:
+
+* **Leaving the BentoBox worlds out of every inventory group does not help.** Groups control which
+  worlds *share* an inventory, not which worlds Multiverse-Inventories handles. It still writes a
+  per-world profile for a world that is in no group.
+* **`/mv remove <world>` does not help either.** Multiverse-Core re-registers BentoBox worlds as
+  they are created, so the removal is undone on the next restart. `auto-import-3rd-party-worlds:
+  false` does not prevent it — that only suppresses the import sweep run when Multiverse-Core
+  starts, which is before BentoBox has created its worlds.
+
+Multiverse-Inventories has no config option to ignore a world, but it does have a bypass
+permission. Enable it in the Multiverse-Inventories `config.yml` (it ships as `false`):
+
+```yml
+share-handling:
+  enable-bypass-permissions: true
+```
+
+Then grant `mvinv.bypass.world.<world>` for each BentoBox world — including its nether and end — to
+every player. With LuckPerms:
+
+```
+lp group default permission set mvinv.bypass.world.bskyblock_world true
+lp group default permission set mvinv.bypass.world.bskyblock_world_nether true
+lp group default permission set mvinv.bypass.world.bskyblock_world_the_end true
+```
+
+Setting the nodes on a group every player inherits (such as `default`) covers new players
+automatically. Repeat for each game mode world you run.
+
+**Operators do not get this permission automatically** — it has to be granted explicitly. Testing
+as an op without it looks exactly like the fix not working.
+
+Use one node per world. Avoid `mvinv.bypass.world.*`, which switches Multiverse-Inventories off for
+every world, including the ones you still want it to manage.
+
 ## Config.yml
 
 The config allows to define which worlds that InvSwitcher should operate, and what aspects should be kept separate.

pom.xml

@@ -65,7 +65,7 @@
         <!-- Do not change unless you want different name for local builds. -->
         <build.number>-LOCAL</build.number>
         <!-- This allows to change between versions. -->
-        <build.version>1.19.1</build.version>
+        <build.version>1.19.2</build.version>
         <!-- Sonar Cloud -->
         <sonar.projectKey>BentoBoxWorld_addon-invSwitcher</sonar.projectKey>
         <sonar.organization>bentobox-world</sonar.organization>
@@ -323,7 +323,7 @@
             <plugin>
                 <groupId>org.jacoco</groupId>
                 <artifactId>jacoco-maven-plugin</artifactId>
-                <version>0.8.12</version>
+                <version>0.8.15</version>
                 <configuration>
                     <append>true</append>
                     <excludes>

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

@@ -474,10 +474,36 @@ public void storeAndSave(Player player, World world, boolean shutdown) {
         }
         if (settings.isStatistics()) {
             String k = settings.isIslandsStatistics() ? islandKey : worldKey;
-            saveStats(store, player, k, shutdown).thenAccept(database::saveObjectAsync);
+            // On shutdown saveStats() gathers synchronously and returns an already-completed
+            // future, so thenAccept runs on this thread and persist() writes before we return.
+            saveStats(store, player, k, shutdown).thenAccept(s -> persist(s, shutdown));
             return;
         }
-        database.saveObjectAsync(store);
+        persist(store, shutdown);
+    }
+
+    /**
+     * Writes the store to the database, synchronously when the server is shutting down.
+     * <p>
+     * Saves are normally asynchronous, but a shutdown save must not be. BentoBox closes its
+     * database immediately after addons are disabled, and players are only kicked afterwards, so an
+     * asynchronous write issued from {@link #saveOnShutdown()} loses the race and is silently
+     * dropped — and the {@code PlayerQuitEvent} that would otherwise save them fires after the
+     * database is already closed.
+     * <p>
+     * The effect was that everything a player did since their last world change went unsaved when
+     * the server stopped. Because {@code PlayerListener.onPlayerJoin} re-applies the stored
+     * inventory on login, the stale snapshot then overwrote the player's real inventory and they
+     * were rolled back to their last world change.
+     * @param store - the store to write
+     * @param shutdown - true if this is a shutdown save, which must be synchronous
+     */
+    private void persist(InventoryStorage store, boolean shutdown) {
+        if (shutdown) {
+            database.saveObject(store);
+        } else {
+            database.saveObjectAsync(store);
+        }
     }
 
     private CompletableFuture<InventoryStorage> saveStats(InventoryStorage store, Player player, String worldName,

src/main/resources/addon.yml

@@ -5,4 +5,30 @@ api-version: 3.17.0
 
 authors: tastybento
 
-softdepend: AcidIsland, BSkyBlock, SkyGrid, CaveBock, AOneBlock
+softdepend: AcidIsland, BSkyBlock, SkyGrid, CaveBock, AOneBlock
+
+# Economy command permissions. These are only usable if options.money is enabled and Vault is
+# installed - InvSwitcher does not register the commands otherwise. The [gamemode] placeholder is
+# expanded by BentoBox into every game mode's permission prefix, e.g. bskyblock.invswitcher.balance
+permissions:
+  '[gamemode].invswitcher.balance':
+    description: Player can use the balance command
+    default: true
+  '[gamemode].invswitcher.pay':
+    description: Player can use the pay command
+    default: true
+  '[gamemode].invswitcher.admin.eco':
+    description: Player can use the admin eco command
+    default: op
+  '[gamemode].invswitcher.admin.eco.balance':
+    description: Player can use the admin eco balance command
+    default: op
+  '[gamemode].invswitcher.admin.eco.give':
+    description: Player can use the admin eco give command
+    default: op
+  '[gamemode].invswitcher.admin.eco.take':
+    description: Player can use the admin eco take command
+    default: op
+  '[gamemode].invswitcher.admin.eco.set':
+    description: Player can use the admin eco set command
+    default: op

src/main/resources/config.yml

@@ -9,6 +9,11 @@ worlds:
 - caveblock-world
 - poseidon_world
 - stranger_world
+- skygrid-world
+- raft_world
+- brix_world
+- parkour_world
+- tradewinds_world
 options:
   # 
   # Per-world settings. Gamemode means Survivial, Creative, etc.

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

@@ -18,6 +18,7 @@
 import static org.mockito.Mockito.when;
 
 import java.io.File;
+import java.lang.reflect.Field;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -54,6 +55,7 @@
 import org.mockito.quality.Strictness;
 
 import world.bentobox.bentobox.BentoBox;
+import world.bentobox.bentobox.database.Database;
 import world.bentobox.bentobox.database.DatabaseSetup.DatabaseType;
 import com.wasteofplastic.invswitcher.dataobjects.InventoryStorage;
 
@@ -905,4 +907,74 @@ void testGetStorageKeyForEventMultipleIslandsNotOwner() {
         }
     }
 
+    // --- Shutdown save tests ---
+
+    /**
+     * Replaces the Store's database with a mock so the save path can be observed.
+     */
+    @SuppressWarnings("unchecked")
+    private Database<InventoryStorage> injectMockDatabase() throws Exception {
+        Database<InventoryStorage> db = mock(Database.class);
+        Field field = Store.class.getDeclaredField("database");
+        field.setAccessible(true);
+        field.set(s, db);
+        return db;
+    }
+
+    /**
+     * A shutdown save must be synchronous. BentoBox closes its database immediately after addons
+     * are disabled, and players are only kicked afterwards, so an asynchronous write issued from
+     * saveOnShutdown() loses the race and is silently dropped. Everything the player did since
+     * their last world change was then lost, and because onPlayerJoin re-applies the stored
+     * inventory on login, the stale snapshot overwrote their real inventory on the next restart.
+     */
+    @Test
+    void testShutdownSaveIsSynchronous() throws Exception {
+        sets.setStatistics(false);
+        sets.setAdvancements(false);
+        Database<InventoryStorage> db = injectMockDatabase();
+
+        try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
+            s.storeAndSave(player, world, true);
+        }
+
+        verify(db).saveObject(any(InventoryStorage.class));
+        verify(db, never()).saveObjectAsync(any(InventoryStorage.class));
+    }
+
+    /**
+     * The statistics branch returns early, so it needs its own check that a shutdown save is
+     * written synchronously.
+     */
+    @Test
+    void testShutdownSaveIsSynchronousWithStatistics() throws Exception {
+        sets.setStatistics(true);
+        sets.setAdvancements(false);
+        Database<InventoryStorage> db = injectMockDatabase();
+
+        try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
+            s.storeAndSave(player, world, true);
+        }
+
+        verify(db).saveObject(any(InventoryStorage.class));
+        verify(db, never()).saveObjectAsync(any(InventoryStorage.class));
+    }
+
+    /**
+     * Normal (non-shutdown) saves must stay asynchronous so they do not block the main thread.
+     */
+    @Test
+    void testNormalSaveStaysAsynchronous() throws Exception {
+        sets.setStatistics(false);
+        sets.setAdvancements(false);
+        Database<InventoryStorage> db = injectMockDatabase();
+
+        try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
+            s.storeAndSave(player, world, false);
+        }
+
+        verify(db).saveObjectAsync(any(InventoryStorage.class));
+        verify(db, never()).saveObject(any(InventoryStorage.class));
+    }
+
 }