diff --git a/pom.xml b/pom.xml index 4767d14..0204a31 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,7 @@ -LOCAL - 1.19.1 + 1.19.2 BentoBoxWorld_addon-invSwitcher bentobox-world @@ -323,7 +323,7 @@ org.jacoco jacoco-maven-plugin - 0.8.12 + 0.8.15 true diff --git a/src/main/java/com/wasteofplastic/invswitcher/Store.java b/src/main/java/com/wasteofplastic/invswitcher/Store.java index a5c69f1..52b41c8 100644 --- a/src/main/java/com/wasteofplastic/invswitcher/Store.java +++ b/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. + *

+ * 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. + *

+ * 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 saveStats(InventoryStorage store, Player player, String worldName, diff --git a/src/main/resources/addon.yml b/src/main/resources/addon.yml index 5cfe563..e681a54 100755 --- a/src/main/resources/addon.yml +++ b/src/main/resources/addon.yml @@ -5,4 +5,30 @@ api-version: 3.17.0 authors: tastybento -softdepend: AcidIsland, BSkyBlock, SkyGrid, CaveBock, AOneBlock \ No newline at end of file +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 diff --git a/src/test/java/com/wasteofplastic/invswitcher/StoreTest.java b/src/test/java/com/wasteofplastic/invswitcher/StoreTest.java index b46eea3..de7b57b 100644 --- a/src/test/java/com/wasteofplastic/invswitcher/StoreTest.java +++ b/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 injectMockDatabase() throws Exception { + Database 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 db = injectMockDatabase(); + + try (MockedStatic 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 db = injectMockDatabase(); + + try (MockedStatic 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 db = injectMockDatabase(); + + try (MockedStatic 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)); + } + }