From 98bc59efc91d2b66396822dc0228b89ec27382b1 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 10 Feb 2024 10:43:52 +0000 Subject: [PATCH] Fixes #4123 - Coal Generator will no longer be locked after researching (#4124) Due to a logic bug in the Legacy storage backend if there was a duplicate ID it would mark it as researched for the first, then see it researched already and remove it on the second. This was happening for the Coal Generator and Bio Reactor here. Both shared he same research ID 173. We're just doing this fix for now until we can move away from the legacy backend (work in progress). --- pom.xml | 13 +++++ .../storage/backend/legacy/LegacyStorage.java | 12 ++++- .../storage/backend/TestLegacyBackend.java | 49 ++++++++++++++++++- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 73c46273c..411d3b015 100644 --- a/pom.xml +++ b/pom.xml @@ -406,8 +406,21 @@ org.jetbrains annotations + + io.papermc.paper + paper-api + + + + + + io.papermc.paper + paper-api + 1.20.4-R0.1-20240205.114523-90 + test + diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java index d7981a546..59b0c82b9 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java @@ -91,7 +91,17 @@ public class LegacyStorage implements Storage { playerFile.setValue("researches." + research.getID(), true); // Remove the research if it's no longer researched - } else if (playerFile.contains("researches." + research.getID())) { + // ---- + // We have a duplicate ID (173) used for both Coal Gen and Bio Reactor + // If you researched the Goal Gen we would remove it on save if you didn't also have the Bio Reactor + // Due to the fact we would set it as researched (true in the branch above) on Coal Gen + // but then go into this branch and remove it if you didn't have Bio Reactor + // Sooooo we're gonna hack this for now while we move away from the Legacy Storage + // Let's make sure the user doesn't have _any_ research with this ID and _then_ remove it + } else if ( + playerFile.contains("researches." + research.getID()) + && !data.getResearches().stream().anyMatch((r) -> r.getID() == research.getID()) + ) { playerFile.setValue("researches." + research.getID(), null); } } diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java index c8e1916f5..98653ee5b 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java @@ -17,6 +17,7 @@ import org.bukkit.configuration.serialization.ConfigurationSerialization; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -52,8 +53,6 @@ class TestLegacyBackend { // within the class isn't being fired (where ItemStack and other classes are registered) ConfigurationSerialization.registerClass(ItemStack.class); ConfigurationSerialization.registerClass(ItemMeta.class); - - setupResearches(); } @AfterAll @@ -62,9 +61,16 @@ class TestLegacyBackend { FileUtils.deleteDirectory(new File("data-storage")); } + @AfterEach + public void cleanup() { + Slimefun.getRegistry().getResearches().clear(); + } + // Test simple loading and saving of player data @Test void testLoadingResearches() throws IOException { + setupResearches(); + // Create a player file which we can load UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -184,6 +190,8 @@ class TestLegacyBackend { @Test void testSavingResearches() throws InterruptedException { + setupResearches(); + // Create a player file which we can load UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -279,6 +287,8 @@ class TestLegacyBackend { // Test realistic situations @Test void testResearchChanges() throws InterruptedException { + setupResearches(); + UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -372,6 +382,41 @@ class TestLegacyBackend { Assertions.assertEquals(1, assertion.getWaypoints().size()); } + @Test + void testDuplicateResearchesDontGetUnResearched() throws InterruptedException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + // Setup initial research + NamespacedKey initialKey = new NamespacedKey(plugin, "test_1"); + Research initialResearch = new Research(initialKey, 1, "Test 1", 100); + initialResearch.register(); + + // Setup duplicate research + // Keep the ID as 1 but change name and key + NamespacedKey duplicateKey = new NamespacedKey(plugin, "test_2"); + Research duplicateResearch = new Research(duplicateKey, 1, "Test 2", 100); + duplicateResearch.register(); + + profile.setResearched(initialResearch, true); + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + // Will have both the initial and duplicate research + Assertions.assertEquals(2, assertion.getResearches().size()); + Assertions.assertTrue(assertion.getResearches().contains(initialResearch)); + Assertions.assertTrue(assertion.getResearches().contains(duplicateResearch)); + } + // Utils private static void setupResearches() { for (int i = 0; i < 10; i++) {