From 21ce6c44be5d2ccc715d686550556cb943d53aba Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 15 Nov 2023 03:21:55 +0000 Subject: [PATCH 01/14] Phase 1 - wip --- .adr-dir | 1 + docs/adr/0001-storage-layer.md | 114 ++++++++++++++++++ docs/adr/README.md | 11 ++ .../slimefun4/api/player/PlayerProfile.java | 39 +++--- .../slimefun4/implementation/Slimefun.java | 12 ++ .../slimefun4/storage/Storage.java | 19 +++ .../slimefun4/storage/data/PlayerData.java | 26 ++++ .../storage/legacy/LegacyStorage.java | 52 ++++++++ 8 files changed, 255 insertions(+), 19 deletions(-) create mode 100644 .adr-dir create mode 100644 docs/adr/0001-storage-layer.md create mode 100644 docs/adr/README.md create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java diff --git a/.adr-dir b/.adr-dir new file mode 100644 index 0000000000..c73b64aed2 --- /dev/null +++ b/.adr-dir @@ -0,0 +1 @@ +docs/adr diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md new file mode 100644 index 0000000000..087f62a26b --- /dev/null +++ b/docs/adr/0001-storage-layer.md @@ -0,0 +1,114 @@ +# 1. Storage layer + +Date: 2023-11-15 + +**DO NOT rely on any APIs introduced until we finish the work completely!** + +## Status + +Proposed + +## Context + +Slimefun has been around for a very long time and due to that, the way we +wrote persistence of data has also been around for a very long time. +While Slimefun has grown, the storage layer has never been adapted. +This means that even all these years later, it's using the same old saving/loading. +This isn't necessarily always bad, however, the way Slimefun has +saved content until now is. + +Today, files are saved as YAML files, which is good for a config format +but terrible for a data store. It has created very large files +that can get corrupted, aren't easy to manage and generally just +take up a lot of space. + +This ADR talks about the future of our data persistence. + +## Decision + +We want to create a new storage layer abstraction and implementations +which will be backwards-compatible but open up new ways of storing data +within Slimefun. The end end goal is we can quickly and easily support +new storage backends (such as binary storage, SQL, etc.) for things like +[PlayerProfile](), [BlockStorage](), etc. + +We also want to be generally more efficient in the way we save and load data. +Today, we HAVE to load way more than is required. +We can improve memory usage by only loading what we need, when we need it. + +We will do this incrementally and at first, in an experimental context. +In that regard, we should aim to minimise the blast radius and lift as much +as possible. + +### Quick changes overview + +* New abstraction over storage to easily support multiple backends. +* Work towards moving away from the legacy YAML based storage. +* Lazy load and save data to more efficiently handle the data life cycle. + +### Implementation details + +There is a new interface called [`Storage`]() which is what all storage +backends will implement. +This will have methods for loading and saving things like +[`PlayerProfile`]() and [`BlockStorage`](). + +Then, backends will implement these +(e.g. [`LegacyStorageBackend`]() (today's YAML situation)) +in order to support these functions. +Not all storage backends are required support each data type. +e.g. SQL may not support [`BlockStorage`](). + + +## Addons + +The goal is that Addons will be able to use implement new storage backends +if they wish and also be extended so they can load/save things as they wish. + +The first few iterations will not focus on Addon support. We want to ensure +this new storage layer will work and supports what we need it to today. + +This ADR will be updated when we get to supporting Addons properly. + +## Considerations + +This will be a big change therefore we will be doing it as incrementally as +possible. + +The current plan looks like this: + +* Phase 1 - Implement legacy data backend for [`PlayerProfile`](). + * We want to load player data using the new storage layer with the current + data system. + * We'll want to monitor for any possible issues and generally refine + how this system should look +* Phase 2 - Implement new experimental binary backend for [`PlayerProfile`](). + * Create a new backend for binary storage + * Implement in an experimental capacity and allow users to opt-in + * Provide a warning that this is **experimental** and there will be bugs. + * Implement new metric for storage backend being used +* Phase 3 - Mark the new backend as stable for [`PlayerProfile`](). + * Mark it as stable and remove the warnings once we're sure things are + working correctly + * Create a migration path for users currently using "legacy". + * **MAYBE** enable by default for new servers? +* Phase 4 - Move [`BlockStorage`]() to new storage layer. + * The big one! We're gonna tackle adding this to BlockStorage. + This will probably be a large change and we'll want to be as + careful as possible here. + * Implement `legacy` and `binary` as experimental storage backends + for BlockStorage and allow users to opt-in + * Provide a warning that this is **experimental** and there will be bugs. +* Phase 5 - Mark the new storage layer as stable for [`BlockStorage`](). + * Mark it as stable and remove the warnings once we're sure things are + working correctly + * Ensure migration path works here too. + * **MAYBE** enable by default for new servers? +* Phase 6 - Finish up and move anything else we want over + * Move over any other data stores we have to the new layer + * We should probably still do experimental -> stable but it should have + less of a lead time. + +## State of work + +* Phase 1: In progress diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000000..1762af11cd --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,11 @@ +# ADR + +An ADR (Architecture Decision Record) is a document describing large changes, why we made them, etc. + +## Making a new ADR + +If you're making a large change to Slimefun, we recommend creating an ADR +in order to document why this is being made and how it works for future contributors. + +Please follow the general format of the former ADRs or use a tool +such as [`adr-tools`](https://github.com/npryce/adr-tools) to generate a new document. \ No newline at end of file diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index 7b975f5702..cbb1ba7abc 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -3,7 +3,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -13,11 +12,13 @@ import java.util.UUID; import java.util.function.Consumer; import java.util.logging.Level; +import java.util.stream.Collectors; import java.util.stream.IntStream; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; import org.apache.commons.lang.Validate; import org.bukkit.Bukkit; import org.bukkit.ChatColor; @@ -29,7 +30,6 @@ import org.bukkit.inventory.ItemStack; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import io.github.bakedlibs.dough.common.ChatColors; import io.github.bakedlibs.dough.common.CommonPatterns; @@ -68,16 +68,18 @@ public class PlayerProfile { private boolean dirty = false; private boolean markedForDeletion = false; - private final Set researches = new HashSet<>(); private final List waypoints = new ArrayList<>(); private final Map backpacks = new HashMap<>(); private final GuideHistory guideHistory = new GuideHistory(this); private final HashedArmorpiece[] armor = { new HashedArmorpiece(), new HashedArmorpiece(), new HashedArmorpiece(), new HashedArmorpiece() }; - protected PlayerProfile(@Nonnull OfflinePlayer p) { + private final PlayerData data; + + protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { this.uuid = p.getUniqueId(); this.name = p.getName(); + this.data = data; configFile = new Config("data-storage/Slimefun/Players/" + uuid.toString() + ".yml"); waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid.toString() + ".yml"); @@ -86,12 +88,6 @@ protected PlayerProfile(@Nonnull OfflinePlayer p) { } private void loadProfileData() { - for (Research research : Slimefun.getRegistry().getResearches()) { - if (configFile.contains("researches." + research.getID())) { - researches.add(research); - } - } - for (String key : waypointsFile.getKeys()) { try { if (waypointsFile.contains(key + ".world") && Bukkit.getWorld(waypointsFile.getString(key + ".world")) != null) { @@ -180,11 +176,9 @@ public void setResearched(@Nonnull Research research, boolean unlock) { dirty = true; if (unlock) { - configFile.setValue("researches." + research.getID(), true); - researches.add(research); + data.getResearches().add(research.getKey()); } else { - configFile.setValue("researches." + research.getID(), null); - researches.remove(research); + data.getResearches().remove(research.getKey()); } } @@ -202,7 +196,7 @@ public boolean hasUnlocked(@Nullable Research research) { return true; } - return !research.isEnabled() || researches.contains(research); + return !research.isEnabled() || data.getResearches().contains(research.getKey()); } /** @@ -228,7 +222,10 @@ public boolean hasUnlockedEverything() { * @return A {@code Hashset} of all Researches this {@link Player} has unlocked */ public @Nonnull Set getResearches() { - return ImmutableSet.copyOf(researches); + return Slimefun.getRegistry().getResearches() + .stream() + .filter((research) -> data.getResearches().contains(research.getKey())) + .collect(Collectors.toUnmodifiableSet()); } /** @@ -346,7 +343,7 @@ private int countNonEmptyResearches(@Nonnull Collection researches) { List titles = Slimefun.getRegistry().getResearchRanks(); int allResearches = countNonEmptyResearches(Slimefun.getRegistry().getResearches()); - float fraction = (float) countNonEmptyResearches(researches) / allResearches; + float fraction = (float) countNonEmptyResearches(getResearches()) / allResearches; int index = (int) (fraction * (titles.size() - 1)); return titles.get(index); @@ -420,7 +417,9 @@ public static boolean get(@Nonnull OfflinePlayer p, @Nonnull Consumer { - AsyncProfileLoadEvent event = new AsyncProfileLoadEvent(new PlayerProfile(p)); + PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId()); + + AsyncProfileLoadEvent event = new AsyncProfileLoadEvent(new PlayerProfile(p, data)); Bukkit.getPluginManager().callEvent(event); Slimefun.getRegistry().getPlayerProfiles().put(uuid, event.getProfile()); @@ -445,7 +444,9 @@ public static boolean request(@Nonnull OfflinePlayer p) { if (!Slimefun.getRegistry().getPlayerProfiles().containsKey(p.getUniqueId())) { // Should probably prevent multiple requests for the same profile in the future Bukkit.getScheduler().runTaskAsynchronously(Slimefun.instance(), () -> { - PlayerProfile pp = new PlayerProfile(p); + PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId()); + + PlayerProfile pp = new PlayerProfile(p, data); Slimefun.getRegistry().getPlayerProfiles().put(p.getUniqueId(), pp); }); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java index e8c7b460a1..aee05193fc 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java @@ -15,6 +15,8 @@ import javax.annotation.Nullable; import javax.annotation.ParametersAreNonnullByDefault; +import io.github.thebusybiscuit.slimefun4.storage.Storage; +import io.github.thebusybiscuit.slimefun4.storage.legacy.LegacyStorage; import org.apache.commons.lang.Validate; import org.bukkit.Bukkit; import org.bukkit.Server; @@ -197,6 +199,9 @@ public final class Slimefun extends JavaPlugin implements SlimefunAddon { private final Config items = new Config(this, "Items.yml"); private final Config researches = new Config(this, "Researches.yml"); + // Data storage + private Storage playerStorage; + // Listeners that need to be accessed elsewhere private final GrapplingHookListener grapplingHookListener = new GrapplingHookListener(); private final BackpackListener backpackListener = new BackpackListener(); @@ -312,6 +317,10 @@ private void onPluginStart() { networkManager = new NetworkManager(networkSize, config.getBoolean("networks.enable-visualizer"), config.getBoolean("networks.delete-excess-items")); + // Data storage + playerStorage = new LegacyStorage(); + logger.log(Level.INFO, "Using legacy storage for player data"); + // Setting up bStats new Thread(metricsService::start, "Slimefun Metrics").start(); @@ -1068,4 +1077,7 @@ public static boolean isNewlyInstalled() { return instance.getServer().getScheduler().runTask(instance, runnable); } + public static @Nonnull Storage getPlayerStorage() { + return instance().playerStorage; + } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java new file mode 100644 index 0000000000..39be06b0ec --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java @@ -0,0 +1,19 @@ +package io.github.thebusybiscuit.slimefun4.storage; + +import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; + +import javax.annotation.concurrent.ThreadSafe; +import java.util.UUID; + +/** + * TODO + * + */ +@ThreadSafe +public interface Storage { + + PlayerData loadPlayerData(UUID uuid); + + void savePlayerData(UUID uuid, PlayerData data); +} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java new file mode 100644 index 0000000000..e7ab51dd42 --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java @@ -0,0 +1,26 @@ +package io.github.thebusybiscuit.slimefun4.storage.data; + +import com.google.common.annotations.Beta; +import org.bukkit.NamespacedKey; + +import java.util.HashSet; +import java.util.Set; + +/** + * The data which backs {@link io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile} + * + * This API is still experimental, it may change without notice. + */ +@Beta +public class PlayerData { + + private final Set researches = new HashSet<>(); + + public PlayerData(Set researches) { + this.researches.addAll(researches); + } + + public Set getResearches() { + return researches; + } +} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java new file mode 100644 index 0000000000..20fd5c95a6 --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java @@ -0,0 +1,52 @@ +package io.github.thebusybiscuit.slimefun4.storage.legacy; + +import io.github.bakedlibs.dough.config.Config; +import io.github.thebusybiscuit.slimefun4.api.researches.Research; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.storage.Storage; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; +import org.bukkit.NamespacedKey; + +import javax.annotation.Nonnull; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; + +public class LegacyStorage implements Storage { + + @Override + public PlayerData loadPlayerData(@Nonnull UUID uuid) { + Config file = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + + Set researches = new HashSet<>(); + + for (Research research : Slimefun.getRegistry().getResearches()) { + if (file.contains("researches." + research.getID())) { + researches.add(research.getKey()); + } + } + + // TODO: + // * Backpacks + // * Waypoints? + + return new PlayerData(researches); + } + + @Override + public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { + Config file = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + + for (NamespacedKey key : data.getResearches()) { + // Legacy data uses IDs, we'll look these up + Optional research = Slimefun.getRegistry().getResearches().stream() + .filter((rs) -> key == rs.getKey()) + .findFirst(); + + research.ifPresent(value -> file.setValue("researches." + value.getID(), true)); + } + + file.save(); + } +} From a15faf03781c524d39c3c4c6890971ec2e86979f Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 16 Dec 2023 05:35:28 +0000 Subject: [PATCH 02/14] Add some tests --- .../slimefun4/api/player/PlayerProfile.java | 4 + .../slimefun4/implementation/Slimefun.java | 5 +- .../slimefun4/storage/Storage.java | 6 +- .../{ => backend}/legacy/LegacyStorage.java | 2 +- .../storage/backend/TestLegacyBackend.java | 114 ++++++++++++++++++ .../slimefun4/test/mocks/MockProfile.java | 8 +- 6 files changed, 133 insertions(+), 6 deletions(-) rename src/main/java/io/github/thebusybiscuit/slimefun4/storage/{ => backend}/legacy/LegacyStorage.java (96%) create mode 100644 src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index cbb1ba7abc..ce62d21046 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -528,6 +528,10 @@ public boolean hasFullProtectionAgainst(@Nonnull ProtectionType type) { return armorCount == 4; } + public PlayerData getPlayerData() { + return this.data; + } + @Override public int hashCode() { return uuid.hashCode(); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java index aee05193fc..12cbc68507 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java @@ -16,7 +16,8 @@ import javax.annotation.ParametersAreNonnullByDefault; import io.github.thebusybiscuit.slimefun4.storage.Storage; -import io.github.thebusybiscuit.slimefun4.storage.legacy.LegacyStorage; +import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; + import org.apache.commons.lang.Validate; import org.bukkit.Bukkit; import org.bukkit.Server; @@ -263,6 +264,8 @@ private void onUnitTestStart() { registry.load(this, config); loadTags(); soundService.reload(false); + // TODO: What do we do here? Use default and let tests override in some way? + playerStorage = new LegacyStorage(); } /** diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java index 39be06b0ec..96f96b8514 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java @@ -1,14 +1,14 @@ package io.github.thebusybiscuit.slimefun4.storage; -import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; import javax.annotation.concurrent.ThreadSafe; import java.util.UUID; /** - * TODO - * + * The {@link Storage} interface is the abstract layer on top of our storage backends. + * Every backend has to implement this interface and has to implement it in a thread-safe way. + * There will be no expectation of running functions in here within the main thread. */ @ThreadSafe public interface Storage { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java similarity index 96% rename from src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java rename to src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java index 20fd5c95a6..b2d936c495 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/legacy/LegacyStorage.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java @@ -1,4 +1,4 @@ -package io.github.thebusybiscuit.slimefun4.storage.legacy; +package io.github.thebusybiscuit.slimefun4.storage.backend.legacy; import io.github.bakedlibs.dough.config.Config; import io.github.thebusybiscuit.slimefun4.api.researches.Research; 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 new file mode 100644 index 0000000000..1e4f639de3 --- /dev/null +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java @@ -0,0 +1,114 @@ +package io.github.thebusybiscuit.slimefun4.storage.backend; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import org.apache.commons.io.FileUtils; +import org.bukkit.Bukkit; +import org.bukkit.NamespacedKey; +import org.bukkit.OfflinePlayer; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import be.seeseemelk.mockbukkit.MockBukkit; +import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; +import io.github.thebusybiscuit.slimefun4.api.researches.Research; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; + +class TestLegacyBackend { + + private static Slimefun plugin; + private static File dataFolder; + + @BeforeAll + public static void load() { + MockBukkit.mock(); + plugin = MockBukkit.load(Slimefun.class); + dataFolder = new File("data-storage/Slimefun/Players"); + dataFolder.mkdirs(); + + setupResearches(); + } + + @AfterAll + public static void unload() throws IOException { + MockBukkit.unmock(); + FileUtils.deleteDirectory(dataFolder); + } + + @Test + void testLoadingBasicPlayerData() throws IOException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + Files.writeString(playerFile.toPath(), """ + researches: + '0': true + '1': true + '2': true + '3': true + '4': true + '5': true + '6': true + '7': true + '8': true + '9': true + """); + + // Load the player data + LegacyStorage storage = new LegacyStorage(); + PlayerData data = storage.loadPlayerData(uuid); + + // Check if the data is correct + Assertions.assertEquals(10, data.getResearches().size()); + for (int i = 0; i < 10; i++) { + Assertions.assertTrue(data.getResearches().contains(Slimefun.getRegistry().getResearches().get(i).getKey())); + } + } + + @Test + void testSavingBasicPlayerData() throws IOException, InterruptedException, ExecutionException { + // 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); + + CompletableFuture future = new CompletableFuture(); + PlayerProfile.get(player, (profile) -> future.complete(profile)); + PlayerProfile profile = future.get(); + + for (Research research : Slimefun.getRegistry().getResearches()) { + profile.setResearched(research, 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); + Assertions.assertEquals(10, assertion.getResearches().size()); + for (int i = 0; i < 10; i++) { + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i).getKey())); + } + } + + // Utils + private static void setupResearches() { + for (int i = 0; i < 10; i++) { + NamespacedKey key = new NamespacedKey(plugin, "test_" + i); + Research research = new Research(key, i, "Test " + i, 100); + research.register(); + } + } +} diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java index 8afad75e12..eac8483b1d 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java @@ -1,15 +1,21 @@ package io.github.thebusybiscuit.slimefun4.test.mocks; +import java.util.Set; + import javax.annotation.Nonnull; import org.bukkit.OfflinePlayer; import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; public class MockProfile extends PlayerProfile { public MockProfile(@Nonnull OfflinePlayer p) { - super(p); + this(p, new PlayerData(Set.of())); } + public MockProfile(@Nonnull OfflinePlayer p, @Nonnull PlayerData data) { + super(p, data); + } } From 5af81f1e37ec2ab6cb67c8fb753c0bd1a13f7728 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sun, 17 Dec 2023 07:27:27 +0000 Subject: [PATCH 03/14] wip --- docs/adr/0001-storage-layer.md | 2 +- .../github/thebusybiscuit/slimefun4/storage/Storage.java | 7 +++++++ .../slimefun4/storage/backend/legacy/LegacyStorage.java | 9 +++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md index 087f62a26b..724039f3bb 100644 --- a/docs/adr/0001-storage-layer.md +++ b/docs/adr/0001-storage-layer.md @@ -81,7 +81,7 @@ The current plan looks like this: * We want to load player data using the new storage layer with the current data system. * We'll want to monitor for any possible issues and generally refine - how this system should look + how this system and should look * Phase 2 - Implement new experimental binary backend for [`PlayerProfile`](). * Create a new backend for binary storage * Implement in an experimental capacity and allow users to opt-in diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java index 96f96b8514..037db2afc3 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java @@ -3,13 +3,20 @@ import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; import javax.annotation.concurrent.ThreadSafe; + +import com.google.common.annotations.Beta; + import java.util.UUID; /** * The {@link Storage} interface is the abstract layer on top of our storage backends. * Every backend has to implement this interface and has to implement it in a thread-safe way. * There will be no expectation of running functions in here within the main thread. + * + *

+ * This API is still experimental, it may change without notice. */ +@Beta @ThreadSafe public interface Storage { 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 b2d936c495..21afa54a42 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 @@ -7,22 +7,27 @@ import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; import org.bukkit.NamespacedKey; +import com.google.common.annotations.Beta; + import javax.annotation.Nonnull; import java.util.HashSet; import java.util.Optional; import java.util.Set; import java.util.UUID; +@Beta public class LegacyStorage implements Storage { @Override public PlayerData loadPlayerData(@Nonnull UUID uuid) { - Config file = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + // Not too sure why this is it's own file + Config waypointFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); Set researches = new HashSet<>(); for (Research research : Slimefun.getRegistry().getResearches()) { - if (file.contains("researches." + research.getID())) { + if (playerFile.contains("researches." + research.getID())) { researches.add(research.getKey()); } } From fc7bc936396d6c399150a7daa0cd78ba70fe5eba Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sun, 17 Dec 2023 08:54:23 +0000 Subject: [PATCH 04/14] Save waypoints --- .gitignore | 1 + .../slimefun4/api/gps/GPSNetwork.java | 2 +- .../slimefun4/api/gps/Waypoint.java | 36 +++++++++--- .../slimefun4/api/player/PlayerProfile.java | 58 +++---------------- .../storage/backend/legacy/LegacyStorage.java | 54 ++++++++++++----- .../slimefun4/storage/data/PlayerData.java | 52 +++++++++++++++-- .../slimefun4/api/gps/TestWaypoints.java | 31 +++++----- .../storage/backend/TestLegacyBackend.java | 4 +- .../slimefun4/test/mocks/MockProfile.java | 2 +- 9 files changed, 146 insertions(+), 94 deletions(-) diff --git a/.gitignore b/.gitignore index fda2f4a052..f025c1e196 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ /.settings/ /.idea/ /.vscode/ +/data-store/ dependency-reduced-pom.xml diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java index d2d9a3c1ee..47574fb2ca 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java @@ -331,7 +331,7 @@ public void addWaypoint(@Nonnull Player p, @Nonnull String name, @Nonnull Locati } } - profile.addWaypoint(new Waypoint(profile, id, event.getLocation(), event.getName())); + profile.addWaypoint(new Waypoint(p.getUniqueId(), id, event.getLocation(), event.getName())); SoundEffect.GPS_NETWORK_ADD_WAYPOINT.playFor(p); Slimefun.getLocalization().sendMessage(p, "gps.waypoint.added", true); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java index 13c8b42561..0cf37b9ed4 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java @@ -1,11 +1,13 @@ package io.github.thebusybiscuit.slimefun4.api.gps; import java.util.Objects; +import java.util.UUID; import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; import org.apache.commons.lang.Validate; +import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.World.Environment; import org.bukkit.entity.Player; @@ -30,7 +32,7 @@ */ public class Waypoint { - private final PlayerProfile profile; + private final UUID uuid; private final String id; private final String name; private final Location location; @@ -48,26 +50,40 @@ public class Waypoint { * The name of this {@link Waypoint} */ @ParametersAreNonnullByDefault - public Waypoint(PlayerProfile profile, String id, Location loc, String name) { - Validate.notNull(profile, "Profile must never be null!"); + public Waypoint(UUID uuid, String id, Location loc, String name) { + Validate.notNull(uuid, "UUID must never be null!"); Validate.notNull(id, "id must never be null!"); Validate.notNull(loc, "Location must never be null!"); Validate.notNull(name, "Name must never be null!"); - this.profile = profile; + this.uuid = uuid; this.id = id; this.location = loc; this.name = name; } /** - * This returns the owner of the {@link Waypoint}. + * This returns the owner {@link UUID} of the {@link Waypoint}. * + * @return The corresponding owner {@link UUID} + */ + @Nonnull + public UUID getUuid() { + return this.uuid; + } + + /** + * This returns the owner of the {@link Waypoint}. + * * @return The corresponding {@link PlayerProfile} + * + * @deprecated Use {@link #getUuid()} instead */ @Nonnull + @Deprecated public PlayerProfile getOwner() { - return profile; + // This is jank and should never actually return null + return PlayerProfile.find(Bukkit.getOfflinePlayer(uuid)).orElse(null); } /** @@ -126,7 +142,7 @@ public ItemStack getIcon() { */ @Override public int hashCode() { - return Objects.hash(profile.getUUID(), id, name, location); + return Objects.hash(this.uuid, this.id, this.name, this.location); } /** @@ -139,7 +155,9 @@ public boolean equals(Object obj) { } Waypoint waypoint = (Waypoint) obj; - return profile.getUUID().equals(waypoint.getOwner().getUUID()) && id.equals(waypoint.getId()) && location.equals(waypoint.getLocation()) && name.equals(waypoint.getName()); + return this.uuid.equals(waypoint.getUuid()) + && id.equals(waypoint.getId()) + && location.equals(waypoint.getLocation()) + && name.equals(waypoint.getName()); } - } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index ce62d21046..02ca4b0a2b 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -1,6 +1,5 @@ package io.github.thebusybiscuit.slimefun4.api.player; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Iterator; @@ -11,8 +10,6 @@ import java.util.Set; import java.util.UUID; import java.util.function.Consumer; -import java.util.logging.Level; -import java.util.stream.Collectors; import java.util.stream.IntStream; import javax.annotation.Nonnull; @@ -22,7 +19,6 @@ import org.apache.commons.lang.Validate; import org.bukkit.Bukkit; import org.bukkit.ChatColor; -import org.bukkit.Location; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; import org.bukkit.command.CommandSender; @@ -30,6 +26,7 @@ import org.bukkit.inventory.ItemStack; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import io.github.bakedlibs.dough.common.ChatColors; import io.github.bakedlibs.dough.common.CommonPatterns; @@ -68,7 +65,6 @@ public class PlayerProfile { private boolean dirty = false; private boolean markedForDeletion = false; - private final List waypoints = new ArrayList<>(); private final Map backpacks = new HashMap<>(); private final GuideHistory guideHistory = new GuideHistory(this); @@ -83,22 +79,6 @@ protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { configFile = new Config("data-storage/Slimefun/Players/" + uuid.toString() + ".yml"); waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid.toString() + ".yml"); - - loadProfileData(); - } - - private void loadProfileData() { - for (String key : waypointsFile.getKeys()) { - try { - if (waypointsFile.contains(key + ".world") && Bukkit.getWorld(waypointsFile.getString(key + ".world")) != null) { - String waypointName = waypointsFile.getString(key + ".name"); - Location loc = waypointsFile.getLocation(key); - waypoints.add(new Waypoint(this, key, loc, waypointName)); - } - } catch (Exception x) { - Slimefun.logger().log(Level.WARNING, x, () -> "Could not load Waypoint \"" + key + "\" for Player \"" + name + '"'); - } - } } /** @@ -176,9 +156,9 @@ public void setResearched(@Nonnull Research research, boolean unlock) { dirty = true; if (unlock) { - data.getResearches().add(research.getKey()); + data.addResearch(research); } else { - data.getResearches().remove(research.getKey()); + data.removeResearch(research); } } @@ -196,7 +176,7 @@ public boolean hasUnlocked(@Nullable Research research) { return true; } - return !research.isEnabled() || data.getResearches().contains(research.getKey()); + return !research.isEnabled() || data.getResearches().contains(research); } /** @@ -222,10 +202,7 @@ public boolean hasUnlockedEverything() { * @return A {@code Hashset} of all Researches this {@link Player} has unlocked */ public @Nonnull Set getResearches() { - return Slimefun.getRegistry().getResearches() - .stream() - .filter((research) -> data.getResearches().contains(research.getKey())) - .collect(Collectors.toUnmodifiableSet()); + return ImmutableSet.copyOf(this.data.getResearches()); } /** @@ -235,7 +212,7 @@ public boolean hasUnlockedEverything() { * @return A {@link List} containing every {@link Waypoint} */ public @Nonnull List getWaypoints() { - return ImmutableList.copyOf(waypoints); + return ImmutableList.copyOf(this.data.getWaypoints()); } /** @@ -246,21 +223,7 @@ public boolean hasUnlockedEverything() { * The {@link Waypoint} to add */ public void addWaypoint(@Nonnull Waypoint waypoint) { - Validate.notNull(waypoint, "Cannot add a 'null' waypoint!"); - - for (Waypoint wp : waypoints) { - if (wp.getId().equals(waypoint.getId())) { - throw new IllegalArgumentException("A Waypoint with that id already exists for this Player"); - } - } - - if (waypoints.size() < 21) { - waypoints.add(waypoint); - - waypointsFile.setValue(waypoint.getId(), waypoint.getLocation()); - waypointsFile.setValue(waypoint.getId() + ".name", waypoint.getName()); - markDirty(); - } + this.data.addWaypoint(waypoint); } /** @@ -271,12 +234,7 @@ public void addWaypoint(@Nonnull Waypoint waypoint) { * The {@link Waypoint} to remove */ public void removeWaypoint(@Nonnull Waypoint waypoint) { - Validate.notNull(waypoint, "Cannot remove a 'null' waypoint!"); - - if (waypoints.remove(waypoint)) { - waypointsFile.setValue(waypoint.getId(), null); - markDirty(); - } + this.data.removeWaypoint(waypoint); } /** 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 21afa54a42..beb65dacd1 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 @@ -1,10 +1,14 @@ package io.github.thebusybiscuit.slimefun4.storage.backend.legacy; import io.github.bakedlibs.dough.config.Config; +import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; import io.github.thebusybiscuit.slimefun4.api.researches.Research; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.storage.Storage; import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; + +import org.bukkit.Bukkit; +import org.bukkit.Location; import org.bukkit.NamespacedKey; import com.google.common.annotations.Beta; @@ -14,6 +18,7 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.logging.Level; @Beta public class LegacyStorage implements Storage { @@ -22,36 +27,57 @@ public class LegacyStorage implements Storage { public PlayerData loadPlayerData(@Nonnull UUID uuid) { Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); // Not too sure why this is it's own file - Config waypointFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); - - Set researches = new HashSet<>(); + Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); + // Load research + Set researches = new HashSet<>(); for (Research research : Slimefun.getRegistry().getResearches()) { if (playerFile.contains("researches." + research.getID())) { - researches.add(research.getKey()); + researches.add(research); + } + } + + // Load waypoints + Set waypoints = new HashSet<>(); + for (String key : waypointsFile.getKeys()) { + try { + if (waypointsFile.contains(key + ".world") && Bukkit.getWorld(waypointsFile.getString(key + ".world")) != null) { + String waypointName = waypointsFile.getString(key + ".name"); + Location loc = waypointsFile.getLocation(key); + waypoints.add(new Waypoint(uuid, key, loc, waypointName)); + } + } catch (Exception x) { + Slimefun.logger().log(Level.WARNING, x, () -> "Could not load Waypoint \"" + key + "\" for Player \"" + uuid + '"'); } } // TODO: // * Backpacks - // * Waypoints? - return new PlayerData(researches); + return new PlayerData(researches, waypoints); } @Override public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { - Config file = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + // Not too sure why this is it's own file + Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); - for (NamespacedKey key : data.getResearches()) { - // Legacy data uses IDs, we'll look these up - Optional research = Slimefun.getRegistry().getResearches().stream() - .filter((rs) -> key == rs.getKey()) - .findFirst(); + // Save research + for (Research research : data.getResearches()) { + // Legacy data uses IDs + playerFile.setValue("researches." + research.getID(), true); + } - research.ifPresent(value -> file.setValue("researches." + value.getID(), true)); + // Save waypoints + for (Waypoint waypoint : data.getWaypoints()) { + // Legacy data uses IDs + waypointsFile.setValue(waypoint.getId(), waypoint.getLocation()); + waypointsFile.setValue(waypoint.getId() + ".name", waypoint.getName()); } - file.save(); + // Save files + playerFile.save(); + waypointsFile.save(); } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java index e7ab51dd42..b07bf7e395 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java @@ -1,26 +1,70 @@ package io.github.thebusybiscuit.slimefun4.storage.data; import com.google.common.annotations.Beta; -import org.bukkit.NamespacedKey; + +import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; +import io.github.thebusybiscuit.slimefun4.api.researches.Research; import java.util.HashSet; import java.util.Set; +import javax.annotation.Nonnull; + +import org.apache.commons.lang.Validate; + /** * The data which backs {@link io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile} * * This API is still experimental, it may change without notice. */ +// TODO: Should we keep this in PlayerProfile? @Beta public class PlayerData { - private final Set researches = new HashSet<>(); + private final Set researches = new HashSet<>(); + private final Set waypoints = new HashSet<>(); - public PlayerData(Set researches) { + public PlayerData(Set researches, Set waypoints) { this.researches.addAll(researches); } - public Set getResearches() { + public Set getResearches() { return researches; } + + public void addResearch(@Nonnull Research research) { + Validate.notNull(research, "Cannot add a 'null' research!"); + researches.add(research); + } + + public void removeResearch(@Nonnull Research research) { + Validate.notNull(research, "Cannot remove a 'null' research!"); + researches.remove(research); + } + + public Set getWaypoints() { + return waypoints; + } + + public void addWaypoint(@Nonnull Waypoint waypoint) { + Validate.notNull(waypoint, "Cannot add a 'null' waypoint!"); + + for (Waypoint wp : waypoints) { + if (wp.getId().equals(waypoint.getId())) { + throw new IllegalArgumentException("A Waypoint with that id already exists for this Player"); + } + } + + // TODO: Figure out why we limit this to 21, it's a weird number + if (waypoints.size() >= 21) { + return; // Also, not sure why this doesn't throw but the one above does... + } + + waypoints.add(waypoint); + } + + public void removeWaypoint(@Nonnull Waypoint waypoint) { + Validate.notNull(waypoint, "Cannot remove a 'null' waypoint!"); + waypoints.remove(waypoint); + } } diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java index d115135ba7..a0de64b14f 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java @@ -1,5 +1,9 @@ package io.github.thebusybiscuit.slimefun4.api.gps; +import java.io.File; +import java.io.IOException; + +import org.apache.commons.io.FileUtils; import org.bukkit.entity.Player; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; @@ -19,16 +23,20 @@ class TestWaypoints { private static ServerMock server; private static Slimefun plugin; + private static File dataFolder; @BeforeAll public static void load() { server = MockBukkit.mock(); plugin = MockBukkit.load(Slimefun.class); + dataFolder = new File("data-storage/Slimefun/waypoints"); + dataFolder.mkdirs(); } @AfterAll - public static void unload() { + public static void unload() throws IOException { MockBukkit.unmock(); + FileUtils.deleteDirectory(dataFolder); } @Test @@ -38,9 +46,8 @@ void testAddWaypointToProfile() throws InterruptedException { PlayerProfile profile = TestUtilities.awaitProfile(player); Assertions.assertTrue(profile.getWaypoints().isEmpty()); - Waypoint waypoint = new Waypoint(profile, "hello", player.getLocation(), "HELLO"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "hello", player.getLocation(), "HELLO"); profile.addWaypoint(waypoint); - Assertions.assertTrue(profile.isDirty()); Assertions.assertThrows(IllegalArgumentException.class, () -> profile.addWaypoint(null)); @@ -55,7 +62,7 @@ void testRemoveWaypointFromProfile() throws InterruptedException { Player player = server.addPlayer(); PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "hello", player.getLocation(), "HELLO"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "hello", player.getLocation(), "HELLO"); profile.addWaypoint(waypoint); Assertions.assertEquals(1, profile.getWaypoints().size()); @@ -76,7 +83,7 @@ void testWaypointAlreadyExisting() throws InterruptedException { Player player = server.addPlayer(); PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "test", player.getLocation(), "Testing"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "test", player.getLocation(), "Testing"); profile.addWaypoint(waypoint); Assertions.assertEquals(1, profile.getWaypoints().size()); @@ -91,7 +98,7 @@ void testTooManyWaypoints() throws InterruptedException { PlayerProfile profile = TestUtilities.awaitProfile(player); for (int i = 0; i < 99; i++) { - Waypoint waypoint = new Waypoint(profile, String.valueOf(i), player.getLocation(), "Test"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), String.valueOf(i), player.getLocation(), "Test"); profile.addWaypoint(waypoint); } @@ -114,11 +121,10 @@ void testWaypointEvent() throws InterruptedException { @DisplayName("Test equal Waypoints being equal") void testWaypointComparison() throws InterruptedException { Player player = server.addPlayer(); - PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "waypoint", player.getLocation(), "Test"); - Waypoint same = new Waypoint(profile, "waypoint", player.getLocation(), "Test"); - Waypoint different = new Waypoint(profile, "waypoint_nope", player.getLocation(), "Test2"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "waypoint", player.getLocation(), "Test"); + Waypoint same = new Waypoint(player.getUniqueId(), "waypoint", player.getLocation(), "Test"); + Waypoint different = new Waypoint(player.getUniqueId(), "waypoint_nope", player.getLocation(), "Test2"); Assertions.assertEquals(waypoint, same); Assertions.assertEquals(waypoint.hashCode(), same.hashCode()); @@ -131,10 +137,9 @@ void testWaypointComparison() throws InterruptedException { @DisplayName("Test Deathpoints being recognized as Deathpoints") void testIsDeathpoint() throws InterruptedException { Player player = server.addPlayer(); - PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "waypoint", player.getLocation(), "Some Waypoint"); - Waypoint deathpoint = new Waypoint(profile, "deathpoint", player.getLocation(), "player:death I died"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "waypoint", player.getLocation(), "Some Waypoint"); + Waypoint deathpoint = new Waypoint(player.getUniqueId(), "deathpoint", player.getLocation(), "player:death I died"); Assertions.assertFalse(waypoint.isDeathpoint()); Assertions.assertTrue(deathpoint.isDeathpoint()); 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 1e4f639de3..1cfaecd2e1 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 @@ -70,7 +70,7 @@ void testLoadingBasicPlayerData() throws IOException { // Check if the data is correct Assertions.assertEquals(10, data.getResearches().size()); for (int i = 0; i < 10; i++) { - Assertions.assertTrue(data.getResearches().contains(Slimefun.getRegistry().getResearches().get(i).getKey())); + Assertions.assertTrue(data.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); } } @@ -99,7 +99,7 @@ void testSavingBasicPlayerData() throws IOException, InterruptedException, Execu PlayerData assertion = storage.loadPlayerData(uuid); Assertions.assertEquals(10, assertion.getResearches().size()); for (int i = 0; i < 10; i++) { - Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i).getKey())); + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); } } diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java index eac8483b1d..6e083e40f9 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java @@ -12,7 +12,7 @@ public class MockProfile extends PlayerProfile { public MockProfile(@Nonnull OfflinePlayer p) { - this(p, new PlayerData(Set.of())); + this(p, new PlayerData(Set.of(), Set.of())); } public MockProfile(@Nonnull OfflinePlayer p, @Nonnull PlayerData data) { From 71d52af8198e59aef5617d64fae7fab0d101be47 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Tue, 19 Dec 2023 03:48:33 +0000 Subject: [PATCH 05/14] Implement backpacks --- .../slimefun4/api/player/PlayerBackpack.java | 65 ++++++++++++- .../slimefun4/api/player/PlayerProfile.java | 19 +--- .../storage/backend/legacy/LegacyStorage.java | 43 +++++++-- .../slimefun4/storage/data/PlayerData.java | 32 ++++++- .../api/profiles/TestPlayerBackpacks.java | 38 +------- .../listeners/TestBackpackListener.java | 13 +-- .../storage/backend/TestLegacyBackend.java | 95 ++++++++++++++++++- .../slimefun4/test/mocks/MockProfile.java | 3 +- 8 files changed, 225 insertions(+), 83 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java index 4960a7c9b3..593c08852f 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java @@ -2,10 +2,13 @@ import java.io.File; import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; +import java.util.UUID; import java.util.concurrent.CompletableFuture; import javax.annotation.Nonnull; +import javax.annotation.ParametersAreNonnullByDefault; import org.bukkit.Bukkit; import org.bukkit.entity.HumanEntity; @@ -34,13 +37,22 @@ public class PlayerBackpack { private static final String CONFIG_PREFIX = "backpacks."; - private final PlayerProfile profile; + private final UUID ownerId; private final int id; - private final Config cfg; + @Deprecated + private PlayerProfile profile; + @Deprecated + private Config cfg; private Inventory inventory; private int size; + private PlayerBackpack(@Nonnull UUID ownerId, int id, int size) { + this.ownerId = ownerId; + this.id = id; + this.size = size; + } + /** * This constructor loads an existing Backpack * @@ -49,6 +61,7 @@ public class PlayerBackpack { * @param id * The id of this Backpack */ + @Deprecated public PlayerBackpack(@Nonnull PlayerProfile profile, int id) { this(profile, id, profile.getConfig().getInt(CONFIG_PREFIX + id + ".size")); @@ -67,11 +80,13 @@ public PlayerBackpack(@Nonnull PlayerProfile profile, int id) { * @param size * The size of this Backpack */ + @Deprecated public PlayerBackpack(@Nonnull PlayerProfile profile, int id, int size) { if (size < 9 || size > 54 || size % 9 != 0) { throw new IllegalArgumentException("Invalid size! Size must be one of: [9, 18, 27, 36, 45, 54]"); } + this.ownerId = profile.getUUID(); this.profile = profile; this.id = id; this.cfg = profile.getConfig(); @@ -97,9 +112,14 @@ public int getId() { * * @return The owning {@link PlayerProfile} */ + @Deprecated @Nonnull public PlayerProfile getOwner() { - return profile; + return profile != null ? profile : PlayerProfile.find(Bukkit.getOfflinePlayer(ownerId)).orElse(null); + } + + public UUID getOwnerId() { + return this.ownerId; } /** @@ -172,7 +192,6 @@ public void setSize(int size) { } this.size = size; - cfg.setValue(CONFIG_PREFIX + id + ".size", size); Inventory inv = Bukkit.createInventory(null, size, "Backpack [" + size + " Slots]"); @@ -188,6 +207,7 @@ public void setSize(int size) { /** * This method will save the contents of this backpack to a {@link File}. */ + @Deprecated public void save() { for (int i = 0; i < size; i++) { cfg.setValue(CONFIG_PREFIX + id + ".contents." + i, inventory.getItem(i)); @@ -198,8 +218,43 @@ public void save() { * This method marks the backpack dirty, it will then be queued for an autosave * using {@link PlayerBackpack#save()} */ + @Deprecated public void markDirty() { - profile.markDirty(); + if (profile != null) { + profile.markDirty(); + } } + private void setContents(int size, HashMap contents) { + if (this.inventory == null) { + this.inventory = Bukkit.createInventory(null, size, "Backpack [" + size + " Slots]"); + } + + for (int i = 0; i < size; i++) { + this.inventory.setItem(i, contents.get(i)); + } + } + + // NTS: The data loading is handled in the storage layer but we handle this specific class here + @ParametersAreNonnullByDefault + public static PlayerBackpack load(UUID ownerId, int id, int size, HashMap contents) { + PlayerBackpack backpack = new PlayerBackpack(ownerId, id, size); + + backpack.setContents(size, contents); + + return backpack; + } + + @ParametersAreNonnullByDefault + public static PlayerBackpack newBackpack(UUID ownerId, int id, int size) { + if (size < 9 || size > 54 || size % 9 != 0) { + throw new IllegalArgumentException("Invalid size! Size must be one of: [9, 18, 27, 36, 45, 54]"); + } + + PlayerBackpack backpack = new PlayerBackpack(ownerId, id, size); + + backpack.setContents(size, new HashMap<>()); + + return backpack; + } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index 02ca4b0a2b..a4bc994d18 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -60,12 +60,10 @@ public class PlayerProfile { private final String name; private final Config configFile; - private final Config waypointsFile; private boolean dirty = false; private boolean markedForDeletion = false; - private final Map backpacks = new HashMap<>(); private final GuideHistory guideHistory = new GuideHistory(this); private final HashedArmorpiece[] armor = { new HashedArmorpiece(), new HashedArmorpiece(), new HashedArmorpiece(), new HashedArmorpiece() }; @@ -78,7 +76,6 @@ protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { this.data = data; configFile = new Config("data-storage/Slimefun/Players/" + uuid.toString() + ".yml"); - waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid.toString() + ".yml"); } /** @@ -133,12 +130,6 @@ public boolean isDirty() { * This method will save the Player's Researches and Backpacks to the hard drive */ public void save() { - for (PlayerBackpack backpack : backpacks.values()) { - backpack.save(); - } - - waypointsFile.save(); - configFile.save(); dirty = false; } @@ -256,8 +247,8 @@ public final void markDirty() { IntStream stream = IntStream.iterate(0, i -> i + 1).filter(i -> !configFile.contains("backpacks." + i + ".size")); int id = stream.findFirst().getAsInt(); - PlayerBackpack backpack = new PlayerBackpack(this, id, size); - backpacks.put(id, backpack); + PlayerBackpack backpack = PlayerBackpack.newBackpack(this.uuid, id, size); + this.data.addBackpack(backpack); return backpack; } @@ -267,14 +258,10 @@ public final void markDirty() { throw new IllegalArgumentException("Backpacks cannot have negative ids!"); } - PlayerBackpack backpack = backpacks.get(id); + PlayerBackpack backpack = data.getBackpack(id); if (backpack != null) { return Optional.of(backpack); - } else if (configFile.contains("backpacks." + id + ".size")) { - backpack = new PlayerBackpack(this, id); - backpacks.put(id, backpack); - return Optional.of(backpack); } return Optional.empty(); 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 beb65dacd1..494c8c2fb4 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 @@ -2,6 +2,7 @@ import io.github.bakedlibs.dough.config.Config; import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; +import io.github.thebusybiscuit.slimefun4.api.player.PlayerBackpack; import io.github.thebusybiscuit.slimefun4.api.researches.Research; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.storage.Storage; @@ -9,13 +10,14 @@ import org.bukkit.Bukkit; import org.bukkit.Location; -import org.bukkit.NamespacedKey; +import org.bukkit.inventory.ItemStack; import com.google.common.annotations.Beta; import javax.annotation.Nonnull; + +import java.util.HashMap; import java.util.HashSet; -import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.logging.Level; @@ -37,6 +39,26 @@ public PlayerData loadPlayerData(@Nonnull UUID uuid) { } } + // Load backpacks + HashMap backpacks = new HashMap<>(); + for (String key : playerFile.getKeys("backpacks")) { + try { + int id = Integer.parseInt(key); + int size = playerFile.getInt("backpacks." + key + ".size"); + + HashMap items = new HashMap<>(); + for (int i = 0; i < size; i++) { + items.put(i, playerFile.getItem("backpacks." + key + ".contents." + i)); + } + + PlayerBackpack backpack = PlayerBackpack.load(uuid, id, size, items); + + backpacks.put(id, backpack); + } catch (Exception x) { + Slimefun.logger().log(Level.WARNING, x, () -> "Could not load Backpack \"" + key + "\" for Player \"" + uuid + '"'); + } + } + // Load waypoints Set waypoints = new HashSet<>(); for (String key : waypointsFile.getKeys()) { @@ -51,10 +73,7 @@ public PlayerData loadPlayerData(@Nonnull UUID uuid) { } } - // TODO: - // * Backpacks - - return new PlayerData(researches, waypoints); + return new PlayerData(researches, backpacks, waypoints); } @Override @@ -69,6 +88,18 @@ public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { playerFile.setValue("researches." + research.getID(), true); } + // Save backpacks + for (PlayerBackpack backpack : data.getBackpacks().values()) { + playerFile.setValue("backpacks." + backpack.getId() + ".size", backpack.getSize()); + + for (int i = 0; i < backpack.getSize(); i++) { + ItemStack item = backpack.getInventory().getItem(i); + if (item != null) { + playerFile.setValue("backpacks." + backpack.getId() + ".contents." + i, item); + } + } + } + // Save waypoints for (Waypoint waypoint : data.getWaypoints()) { // Legacy data uses IDs diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java index b07bf7e395..8615b6ee5f 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java @@ -3,9 +3,12 @@ import com.google.common.annotations.Beta; import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; +import io.github.thebusybiscuit.slimefun4.api.player.PlayerBackpack; import io.github.thebusybiscuit.slimefun4.api.researches.Research; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; @@ -22,10 +25,13 @@ public class PlayerData { private final Set researches = new HashSet<>(); + private final Map backpacks = new HashMap<>(); private final Set waypoints = new HashSet<>(); - public PlayerData(Set researches, Set waypoints) { + public PlayerData(Set researches, Map backpacks, Set waypoints) { this.researches.addAll(researches); + this.backpacks.putAll(backpacks); + this.waypoints.addAll(waypoints); } public Set getResearches() { @@ -42,6 +48,26 @@ public void removeResearch(@Nonnull Research research) { researches.remove(research); } + @Nonnull + public Map getBackpacks() { + return backpacks; + } + + @Nonnull + public PlayerBackpack getBackpack(int id) { + return backpacks.get(id); + } + + public void addBackpack(@Nonnull PlayerBackpack backpack) { + Validate.notNull(backpack, "Cannot add a 'null' backpack!"); + backpacks.put(backpack.getId(), backpack); + } + + public void removeBackpack(@Nonnull PlayerBackpack backpack) { + Validate.notNull(backpack, "Cannot remove a 'null' backpack!"); + backpacks.remove(backpack.getId()); + } + public Set getWaypoints() { return waypoints; } @@ -55,9 +81,9 @@ public void addWaypoint(@Nonnull Waypoint waypoint) { } } - // TODO: Figure out why we limit this to 21, it's a weird number + // Limited to 21 due to limited UI space and no pagination if (waypoints.size() >= 21) { - return; // Also, not sure why this doesn't throw but the one above does... + return; // not sure why this doesn't throw but the one above does... } waypoints.add(waypoint); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java index 3c1ae7175c..07f69761e0 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java @@ -2,9 +2,7 @@ import java.util.Optional; -import org.bukkit.Material; import org.bukkit.entity.Player; -import org.bukkit.inventory.ItemStack; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -39,16 +37,12 @@ public static void unload() { void testCreateBackpack() throws InterruptedException { Player player = server.addPlayer(); PlayerProfile profile = TestUtilities.awaitProfile(player); - Assertions.assertFalse(profile.isDirty()); PlayerBackpack backpack = profile.createBackpack(18); Assertions.assertNotNull(backpack); - // Creating a backpack should mark profiles as dirty - Assertions.assertTrue(profile.isDirty()); - - Assertions.assertEquals(profile, backpack.getOwner()); + Assertions.assertEquals(player.getUniqueId(), backpack.getOwnerId()); Assertions.assertEquals(18, backpack.getSize()); Assertions.assertEquals(18, backpack.getInventory().getSize()); } @@ -71,7 +65,6 @@ void testChangeSize() throws InterruptedException { backpack.setSize(27); Assertions.assertEquals(27, backpack.getSize()); - Assertions.assertTrue(profile.isDirty()); } @Test @@ -90,33 +83,4 @@ void testGetBackpackById() throws InterruptedException { Assertions.assertFalse(profile.getBackpack(500).isPresent()); } - - @Test - @DisplayName("Test loading a backpack from file") - void testLoadBackpackFromFile() throws InterruptedException { - Player player = server.addPlayer(); - PlayerProfile profile = TestUtilities.awaitProfile(player); - - profile.getConfig().setValue("backpacks.50.size", 27); - - for (int i = 0; i < 27; i++) { - profile.getConfig().setValue("backpacks.50.contents." + i, new ItemStack(Material.DIAMOND)); - } - - Optional optional = profile.getBackpack(50); - Assertions.assertTrue(optional.isPresent()); - - PlayerBackpack backpack = optional.get(); - Assertions.assertEquals(50, backpack.getId()); - Assertions.assertEquals(27, backpack.getSize()); - Assertions.assertEquals(-1, backpack.getInventory().firstEmpty()); - - backpack.getInventory().setItem(1, new ItemStack(Material.NETHER_STAR)); - - Assertions.assertEquals(new ItemStack(Material.DIAMOND), profile.getConfig().getItem("backpacks.50.contents.1")); - - // Saving should write it to the Config file - backpack.save(); - Assertions.assertEquals(new ItemStack(Material.NETHER_STAR), profile.getConfig().getItem("backpacks.50.contents.1")); - } } diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java index 01e83b9e71..443d47c3c7 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java @@ -13,7 +13,6 @@ import org.bukkit.event.inventory.ClickType; import org.bukkit.event.inventory.InventoryAction; import org.bukkit.event.inventory.InventoryClickEvent; -import org.bukkit.event.inventory.InventoryCloseEvent; import org.bukkit.event.inventory.InventoryType.SlotType; import org.bukkit.event.player.PlayerDropItemEvent; import org.bukkit.inventory.Inventory; @@ -119,7 +118,7 @@ void testSetId() throws InterruptedException { Assertions.assertEquals(ChatColor.GRAY + "ID: " + player.getUniqueId() + "#" + id, item.getItemMeta().getLore().get(2)); PlayerBackpack backpack = awaitBackpack(item); - Assertions.assertEquals(player.getUniqueId(), backpack.getOwner().getUUID()); + Assertions.assertEquals(player.getUniqueId(), backpack.getOwnerId()); Assertions.assertEquals(id, backpack.getId()); } @@ -132,16 +131,6 @@ void testOpenBackpack() throws InterruptedException { Assertions.assertEquals(backpack.getInventory(), view.getTopInventory()); } - @Test - @DisplayName("Test backpacks being marked dirty on close") - void testCloseBackpack() throws InterruptedException { - Player player = server.addPlayer(); - PlayerBackpack backpack = openMockBackpack(player, "TEST_CLOSE_BACKPACK", 27); - listener.onClose(new InventoryCloseEvent(player.getOpenInventory())); - - Assertions.assertTrue(backpack.getOwner().isDirty()); - } - @Test @DisplayName("Test backpacks not disturbing normal item dropping") void testBackpackDropNormalItem() throws InterruptedException { 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 1cfaecd2e1..3b1c0bc658 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 @@ -11,6 +11,9 @@ import org.bukkit.Bukkit; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; +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.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -35,17 +38,23 @@ public static void load() { dataFolder = new File("data-storage/Slimefun/Players"); dataFolder.mkdirs(); + // Not too sure why this is needed, we don't use it elsewhere, it should just use the ItemStack serialization + // My guess is MockBukkit isn't loading the ConfigurationSerialization class therefore the static block + // within the class isn't being fired (where ItemStack and other classes are registered) + ConfigurationSerialization.registerClass(ItemStack.class); + ConfigurationSerialization.registerClass(ItemMeta.class); + setupResearches(); } @AfterAll public static void unload() throws IOException { MockBukkit.unmock(); - FileUtils.deleteDirectory(dataFolder); + FileUtils.deleteDirectory(new File("data-storage")); } @Test - void testLoadingBasicPlayerData() throws IOException { + void testLoadingResearches() throws IOException { // Create a player file which we can load UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -74,8 +83,59 @@ void testLoadingBasicPlayerData() throws IOException { } } + // There's some issues with deserializing items in tests, I spent quite a while debugging this + // and didn't really get anywhere. So commenting this out for now. + /* + @Test + void testLoadingBackpacks() throws IOException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + Files.writeString(playerFile.toPath(), """ + backpacks: + '0': + size: 9 + contents: + '0': + ==: org.bukkit.inventory.ItemStack + v: 1 + type: IRON_BLOCK + meta: + ==: org.bukkit.inventory.meta.ItemMeta + enchants: {} + damage: 0 + persistentDataContainer: + slimefun:slimefun_item: TEST + displayName: ยง6Test block + itemFlags: !!set {} + unbreakable: false + repairCost: 0 + """); + + // Load the player data + LegacyStorage storage = new LegacyStorage(); + PlayerData data = storage.loadPlayerData(uuid); + + // Check if the data is correct + Assertions.assertEquals(1, data.getBackpacks().size()); + Assertions.assertEquals(9, data.getBackpacks().get(0).getSize()); + + // Validate item deserialization + System.out.println( + Arrays.stream(data.getBackpack(0).getInventory().getContents()) + .map((item) -> item == null ? "null" : item.getType().name()) + .collect(Collectors.joining(", ")) + ); + ItemStack stack = data.getBackpack(0).getInventory().getItem(0); + Assertions.assertNotNull(stack); + Assertions.assertEquals("IRON_BLOCK", stack.getType().name()); + Assertions.assertEquals(1, stack.getAmount()); + Assertions.assertEquals(ChatColor.GREEN + "Test block", stack.getItemMeta().getDisplayName()); + } + */ + @Test - void testSavingBasicPlayerData() throws IOException, InterruptedException, ExecutionException { + void testSavingResearches() throws IOException, InterruptedException, ExecutionException { // Create a player file which we can load UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -103,6 +163,35 @@ void testSavingBasicPlayerData() throws IOException, InterruptedException, Execu } } + // There's some issues with deserializing items in tests, I spent quite a while debugging this + // and didn't really get anywhere. So commenting this out for now. + /* + @Test + void testSavingBackpacks() throws IOException, InterruptedException, ExecutionException { + // 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); + + CompletableFuture future = new CompletableFuture(); + PlayerProfile.get(player, (profile) -> future.complete(profile)); + PlayerProfile profile = future.get(); + + PlayerBackpack backpack = profile.createBackpack(9); + backpack.getInventory().addItem(SlimefunItems.AIR_RUNE); + + // 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); + Assertions.assertEquals(1, assertion.getBackpacks().size()); + } + */ + // Utils private static void setupResearches() { for (int i = 0; i < 10; i++) { diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java index 6e083e40f9..9a7837d00a 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java @@ -1,5 +1,6 @@ package io.github.thebusybiscuit.slimefun4.test.mocks; +import java.util.HashMap; import java.util.Set; import javax.annotation.Nonnull; @@ -12,7 +13,7 @@ public class MockProfile extends PlayerProfile { public MockProfile(@Nonnull OfflinePlayer p) { - this(p, new PlayerData(Set.of(), Set.of())); + this(p, new PlayerData(Set.of(), new HashMap<>(), Set.of())); } public MockProfile(@Nonnull OfflinePlayer p, @Nonnull PlayerData data) { From 7ac9aab2d07591947c384114e80e3d178508ae59 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Tue, 19 Dec 2023 04:12:56 +0000 Subject: [PATCH 06/14] Add tests for waypoints --- .../slimefun4/api/gps/Waypoint.java | 22 ++-- .../storage/backend/legacy/LegacyStorage.java | 2 + .../storage/backend/TestLegacyBackend.java | 111 +++++++++++++++--- 3 files changed, 110 insertions(+), 25 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java index 0cf37b9ed4..b7d8580c70 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java @@ -32,7 +32,7 @@ */ public class Waypoint { - private final UUID uuid; + private final UUID ownerId; private final String id; private final String name; private final Location location; @@ -40,8 +40,8 @@ public class Waypoint { /** * This constructs a new {@link Waypoint} object. * - * @param profile - * The owning {@link PlayerProfile} + * @param ownerId + * The owning {@link Player}'s {@link UUID} * @param id * The unique id for this {@link Waypoint} * @param loc @@ -50,13 +50,13 @@ public class Waypoint { * The name of this {@link Waypoint} */ @ParametersAreNonnullByDefault - public Waypoint(UUID uuid, String id, Location loc, String name) { - Validate.notNull(uuid, "UUID must never be null!"); + public Waypoint(UUID ownerId, String id, Location loc, String name) { + Validate.notNull(ownerId, "owner ID must never be null!"); Validate.notNull(id, "id must never be null!"); Validate.notNull(loc, "Location must never be null!"); Validate.notNull(name, "Name must never be null!"); - this.uuid = uuid; + this.ownerId = ownerId; this.id = id; this.location = loc; this.name = name; @@ -68,8 +68,8 @@ public Waypoint(UUID uuid, String id, Location loc, String name) { * @return The corresponding owner {@link UUID} */ @Nonnull - public UUID getUuid() { - return this.uuid; + public UUID getOwnerId() { + return this.ownerId; } /** @@ -83,7 +83,7 @@ public UUID getUuid() { @Deprecated public PlayerProfile getOwner() { // This is jank and should never actually return null - return PlayerProfile.find(Bukkit.getOfflinePlayer(uuid)).orElse(null); + return PlayerProfile.find(Bukkit.getOfflinePlayer(ownerId)).orElse(null); } /** @@ -142,7 +142,7 @@ public ItemStack getIcon() { */ @Override public int hashCode() { - return Objects.hash(this.uuid, this.id, this.name, this.location); + return Objects.hash(this.ownerId, this.id, this.name, this.location); } /** @@ -155,7 +155,7 @@ public boolean equals(Object obj) { } Waypoint waypoint = (Waypoint) obj; - return this.uuid.equals(waypoint.getUuid()) + return this.ownerId.equals(waypoint.getOwnerId()) && id.equals(waypoint.getId()) && location.equals(waypoint.getLocation()) && name.equals(waypoint.getName()); 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 494c8c2fb4..9310c236cb 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 @@ -16,6 +16,8 @@ import javax.annotation.Nonnull; +import java.io.IOException; +import java.nio.file.Files; import java.util.HashMap; import java.util.HashSet; import java.util.Set; 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 3b1c0bc658..804f24306a 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 @@ -4,13 +4,15 @@ import java.io.IOException; import java.nio.file.Files; import java.util.UUID; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; import org.apache.commons.io.FileUtils; import org.bukkit.Bukkit; +import org.bukkit.Location; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; +import org.bukkit.World; +import org.bukkit.WorldCreator; +import org.bukkit.World.Environment; import org.bukkit.configuration.serialization.ConfigurationSerialization; import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; @@ -20,23 +22,30 @@ import org.junit.jupiter.api.Test; import be.seeseemelk.mockbukkit.MockBukkit; +import be.seeseemelk.mockbukkit.ServerMock; +import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; import io.github.thebusybiscuit.slimefun4.api.researches.Research; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; +import io.github.thebusybiscuit.slimefun4.test.TestUtilities; +import net.md_5.bungee.api.ChatColor; class TestLegacyBackend { + private static ServerMock server; private static Slimefun plugin; - private static File dataFolder; @BeforeAll public static void load() { - MockBukkit.mock(); + server = MockBukkit.mock(); plugin = MockBukkit.load(Slimefun.class); - dataFolder = new File("data-storage/Slimefun/Players"); - dataFolder.mkdirs(); + + File playerFolder = new File("data-storage/Slimefun/Players"); + playerFolder.mkdirs(); + File waypointFolder = new File("data-storage/Slimefun/waypoints"); + waypointFolder.mkdirs(); // Not too sure why this is needed, we don't use it elsewhere, it should just use the ItemStack serialization // My guess is MockBukkit isn't loading the ConfigurationSerialization class therefore the static block @@ -135,16 +144,52 @@ void testLoadingBackpacks() throws IOException { */ @Test - void testSavingResearches() throws IOException, InterruptedException, ExecutionException { + void testLoadingWaypoints() throws IOException { + // Create mock world + server.createWorld(WorldCreator.name("world").environment(Environment.NORMAL)); + + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File waypointFile = new File("data-storage/Slimefun/waypoints/" + uuid + ".yml"); + Files.writeString(waypointFile.toPath(), """ + TEST: + x: -173.0 + y: 75.0 + z: -11.0 + pitch: 0.0 + yaw: 178.0 + world: world + name: test + """); + + // Load the player data + LegacyStorage storage = new LegacyStorage(); + PlayerData data = storage.loadPlayerData(uuid); + + // Check if the data is correct + Assertions.assertEquals(1, data.getWaypoints().size()); + + // Validate waypoint deserialization + Waypoint waypoint = data.getWaypoints().iterator().next(); + + Assertions.assertEquals("test", waypoint.getName()); + Assertions.assertEquals(-173.0, waypoint.getLocation().getX()); + Assertions.assertEquals(75.0, waypoint.getLocation().getY()); + Assertions.assertEquals(-11.0, waypoint.getLocation().getZ()); + Assertions.assertEquals(178.0, waypoint.getLocation().getYaw()); + Assertions.assertEquals(0.0, waypoint.getLocation().getPitch()); + Assertions.assertEquals("world", waypoint.getLocation().getWorld().getName()); + } + + @Test + void testSavingResearches() 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); - CompletableFuture future = new CompletableFuture(); - PlayerProfile.get(player, (profile) -> future.complete(profile)); - PlayerProfile profile = future.get(); + PlayerProfile profile = TestUtilities.awaitProfile(player); for (Research research : Slimefun.getRegistry().getResearches()) { profile.setResearched(research, true); @@ -167,16 +212,14 @@ void testSavingResearches() throws IOException, InterruptedException, ExecutionE // and didn't really get anywhere. So commenting this out for now. /* @Test - void testSavingBackpacks() throws IOException, InterruptedException, ExecutionException { + void testSavingBackpacks() 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); - CompletableFuture future = new CompletableFuture(); - PlayerProfile.get(player, (profile) -> future.complete(profile)); - PlayerProfile profile = future.get(); + PlayerProfile profile = TestUtilities.awaitProfile(player); PlayerBackpack backpack = profile.createBackpack(9); backpack.getInventory().addItem(SlimefunItems.AIR_RUNE); @@ -192,6 +235,46 @@ void testSavingBackpacks() throws IOException, InterruptedException, ExecutionEx } */ + @Test + void testSavingWaypoints() throws InterruptedException { + // Create mock world + World world = server.createWorld(WorldCreator.name("world").environment(Environment.NORMAL)); + + // 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); + + profile.addWaypoint(new Waypoint( + player.getUniqueId(), + "test", + new Location(world, 1, 2, 3, 4, 5), + ChatColor.GREEN + "Test waypoint") + ); + + // 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); + Assertions.assertEquals(1, assertion.getWaypoints().size()); + + // Validate waypoint deserialization + Waypoint waypoint = assertion.getWaypoints().iterator().next(); + + Assertions.assertEquals(ChatColor.GREEN + "Test waypoint", waypoint.getName()); + Assertions.assertEquals(1, waypoint.getLocation().getX()); + Assertions.assertEquals(2, waypoint.getLocation().getY()); + Assertions.assertEquals(3, waypoint.getLocation().getZ()); + Assertions.assertEquals(4, waypoint.getLocation().getYaw()); + Assertions.assertEquals(5, waypoint.getLocation().getPitch()); + Assertions.assertEquals("world", waypoint.getLocation().getWorld().getName()); + } + // Utils private static void setupResearches() { for (int i = 0; i < 10; i++) { From 0d103a96f4b48f994085d7060916ee1c1b31ef99 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Tue, 19 Dec 2023 04:23:36 +0000 Subject: [PATCH 07/14] Changes to ADR --- docs/adr/0001-storage-layer.md | 36 +++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md index 724039f3bb..8923eaa947 100644 --- a/docs/adr/0001-storage-layer.md +++ b/docs/adr/0001-storage-layer.md @@ -14,13 +14,17 @@ Slimefun has been around for a very long time and due to that, the way we wrote persistence of data has also been around for a very long time. While Slimefun has grown, the storage layer has never been adapted. This means that even all these years later, it's using the same old saving/loading. -This isn't necessarily always bad, however, the way Slimefun has -saved content until now is. +This isn't necessarily always bad, however, as Slimefun has grown both in terms of content +and the servers using it - we've seen some issues. -Today, files are saved as YAML files, which is good for a config format -but terrible for a data store. It has created very large files -that can get corrupted, aren't easy to manage and generally just -take up a lot of space. +Today, files are saved as YAML files (sometimes with just a JSON object per line), +which is good for a config format but not good for a data store. It can create very large files +that can get corrupted, the way we've been saving data often means loading it all at once as well +rather than lazy-loading and generally isn't very performant. + +For a long time we've been talking about rewriting our data storage in multiple forms +(you may have seen this referenced for "BlockStorage rewrite" or "SQL for PlayerProfiles", etc.). +Now is the time we start to do this, this will be a very large change and will not be done quickly or rushed. This ADR talks about the future of our data persistence. @@ -30,10 +34,10 @@ We want to create a new storage layer abstraction and implementations which will be backwards-compatible but open up new ways of storing data within Slimefun. The end end goal is we can quickly and easily support new storage backends (such as binary storage, SQL, etc.) for things like -[PlayerProfile](), [BlockStorage](), etc. +[PlayerProfile](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java), [BlockStorage](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java), etc. We also want to be generally more efficient in the way we save and load data. -Today, we HAVE to load way more than is required. +Today, we can to load way more than is required. We can improve memory usage by only loading what we need, when we need it. We will do this incrementally and at first, in an experimental context. @@ -51,13 +55,13 @@ as possible. There is a new interface called [`Storage`]() which is what all storage backends will implement. This will have methods for loading and saving things like -[`PlayerProfile`]() and [`BlockStorage`](). +[`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java) and [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). Then, backends will implement these -(e.g. [`LegacyStorageBackend`]() (today's YAML situation)) +(e.g. [`LegacyStorageBackend`](TBD) (today's YAML situation)) in order to support these functions. Not all storage backends are required support each data type. -e.g. SQL may not support [`BlockStorage`](). +e.g. SQL may not support [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). ## Addons @@ -77,29 +81,29 @@ possible. The current plan looks like this: -* Phase 1 - Implement legacy data backend for [`PlayerProfile`](). +* Phase 1 - Implement legacy data backend for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). * We want to load player data using the new storage layer with the current data system. * We'll want to monitor for any possible issues and generally refine how this system and should look -* Phase 2 - Implement new experimental binary backend for [`PlayerProfile`](). +* Phase 2 - Implement new experimental binary backend for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). * Create a new backend for binary storage * Implement in an experimental capacity and allow users to opt-in * Provide a warning that this is **experimental** and there will be bugs. * Implement new metric for storage backend being used -* Phase 3 - Mark the new backend as stable for [`PlayerProfile`](). +* Phase 3 - Mark the new backend as stable for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). * Mark it as stable and remove the warnings once we're sure things are working correctly * Create a migration path for users currently using "legacy". * **MAYBE** enable by default for new servers? -* Phase 4 - Move [`BlockStorage`]() to new storage layer. +* Phase 4 - Move [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java) to new storage layer. * The big one! We're gonna tackle adding this to BlockStorage. This will probably be a large change and we'll want to be as careful as possible here. * Implement `legacy` and `binary` as experimental storage backends for BlockStorage and allow users to opt-in * Provide a warning that this is **experimental** and there will be bugs. -* Phase 5 - Mark the new storage layer as stable for [`BlockStorage`](). +* Phase 5 - Mark the new storage layer as stable for [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). * Mark it as stable and remove the warnings once we're sure things are working correctly * Ensure migration path works here too. From 5c14b5f4d5aea36db8244c7f1d551797a3ad4795 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Tue, 19 Dec 2023 14:25:19 +0000 Subject: [PATCH 08/14] Small changes --- docs/adr/0001-storage-layer.md | 2 ++ .../thebusybiscuit/slimefun4/implementation/Slimefun.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md index 8923eaa947..d8f426893e 100644 --- a/docs/adr/0001-storage-layer.md +++ b/docs/adr/0001-storage-layer.md @@ -78,6 +78,8 @@ This ADR will be updated when we get to supporting Addons properly. This will be a big change therefore we will be doing it as incrementally as possible. +Changes will be tested while in the PR stage and merged into the Dev releases when possible. +We may do an experimental release if required. The current plan looks like this: diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java index 12cbc68507..8667eed682 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java @@ -264,7 +264,8 @@ private void onUnitTestStart() { registry.load(this, config); loadTags(); soundService.reload(false); - // TODO: What do we do here? Use default and let tests override in some way? + // TODO: What do we do if tests want to use another storage backend (e.g. testing new feature on legacy + sql)? + // Do we have a way to override this? playerStorage = new LegacyStorage(); } From b13392a938fdcf7997532dc39cb1f8211ddf4064 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 20 Dec 2023 04:31:48 +0000 Subject: [PATCH 09/14] Fix englandish and some small changes to UUID in PlayerProfile --- docs/adr/0001-storage-layer.md | 6 +++--- .../slimefun4/api/player/PlayerProfile.java | 19 +++++++++---------- .../storage/backend/legacy/LegacyStorage.java | 6 ++---- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md index d8f426893e..1d04549295 100644 --- a/docs/adr/0001-storage-layer.md +++ b/docs/adr/0001-storage-layer.md @@ -37,7 +37,7 @@ new storage backends (such as binary storage, SQL, etc.) for things like [PlayerProfile](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java), [BlockStorage](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java), etc. We also want to be generally more efficient in the way we save and load data. -Today, we can to load way more than is required. +Today, we load way more than is required. We can improve memory usage by only loading what we need, when we need it. We will do this incrementally and at first, in an experimental context. @@ -66,7 +66,7 @@ e.g. SQL may not support [`BlockStorage`](https://github.com/Slimefun/Slimefun4/ ## Addons -The goal is that Addons will be able to use implement new storage backends +The goal is that Addons will be able to use and implement new storage backends if they wish and also be extended so they can load/save things as they wish. The first few iterations will not focus on Addon support. We want to ensure @@ -87,7 +87,7 @@ The current plan looks like this: * We want to load player data using the new storage layer with the current data system. * We'll want to monitor for any possible issues and generally refine - how this system and should look + how this system should look * Phase 2 - Implement new experimental binary backend for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). * Create a new backend for binary storage * Implement in an experimental capacity and allow users to opt-in diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index a4bc994d18..8ba4b78b10 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -1,10 +1,8 @@ package io.github.thebusybiscuit.slimefun4.api.player; import java.util.Collection; -import java.util.HashMap; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.OptionalInt; import java.util.Set; @@ -56,7 +54,7 @@ */ public class PlayerProfile { - private final UUID uuid; + private final UUID ownerId; private final String name; private final Config configFile; @@ -71,11 +69,11 @@ public class PlayerProfile { private final PlayerData data; protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { - this.uuid = p.getUniqueId(); + this.ownerId = p.getUniqueId(); this.name = p.getName(); this.data = data; - configFile = new Config("data-storage/Slimefun/Players/" + uuid.toString() + ".yml"); + configFile = new Config("data-storage/Slimefun/Players/" + ownerId.toString() + ".yml"); } /** @@ -104,7 +102,7 @@ protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { * @return The {@link UUID} of our {@link PlayerProfile} */ public @Nonnull UUID getUUID() { - return uuid; + return ownerId; } /** @@ -130,6 +128,7 @@ public boolean isDirty() { * This method will save the Player's Researches and Backpacks to the hard drive */ public void save() { + Slimefun.getPlayerStorage().savePlayerData(this.ownerId, this.data); dirty = false; } @@ -247,7 +246,7 @@ public final void markDirty() { IntStream stream = IntStream.iterate(0, i -> i + 1).filter(i -> !configFile.contains("backpacks." + i + ".size")); int id = stream.findFirst().getAsInt(); - PlayerBackpack backpack = PlayerBackpack.newBackpack(this.uuid, id, size); + PlayerBackpack backpack = PlayerBackpack.newBackpack(this.ownerId, id, size); this.data.addBackpack(backpack); return backpack; @@ -479,17 +478,17 @@ public PlayerData getPlayerData() { @Override public int hashCode() { - return uuid.hashCode(); + return ownerId.hashCode(); } @Override public boolean equals(Object obj) { - return obj instanceof PlayerProfile profile && uuid.equals(profile.uuid); + return obj instanceof PlayerProfile profile && ownerId.equals(profile.ownerId); } @Override public String toString() { - return "PlayerProfile {" + uuid + "}"; + return "PlayerProfile {" + ownerId + "}"; } } 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 9310c236cb..535eabc9b9 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 @@ -16,8 +16,6 @@ import javax.annotation.Nonnull; -import java.io.IOException; -import java.nio.file.Files; import java.util.HashMap; import java.util.HashSet; import java.util.Set; @@ -30,7 +28,7 @@ public class LegacyStorage implements Storage { @Override public PlayerData loadPlayerData(@Nonnull UUID uuid) { Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); - // Not too sure why this is it's own file + // Not too sure why this is its own file Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); // Load research @@ -81,7 +79,7 @@ public PlayerData loadPlayerData(@Nonnull UUID uuid) { @Override public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); - // Not too sure why this is it's own file + // Not too sure why this is its own file Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); // Save research From fdbb8d12e68545f093ce6329ea40b46b2dc9cb0d Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 20 Dec 2023 21:09:53 +0000 Subject: [PATCH 10/14] Fix saving of player data in a few cases --- .../thebusybiscuit/slimefun4/api/player/PlayerProfile.java | 5 +++++ .../slimefun4/storage/backend/legacy/LegacyStorage.java | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index 8ba4b78b10..c51888a1d0 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -214,6 +214,7 @@ public boolean hasUnlockedEverything() { */ public void addWaypoint(@Nonnull Waypoint waypoint) { this.data.addWaypoint(waypoint); + markDirty(); } /** @@ -225,6 +226,7 @@ public void addWaypoint(@Nonnull Waypoint waypoint) { */ public void removeWaypoint(@Nonnull Waypoint waypoint) { this.data.removeWaypoint(waypoint); + markDirty(); } /** @@ -249,6 +251,8 @@ public final void markDirty() { PlayerBackpack backpack = PlayerBackpack.newBackpack(this.ownerId, id, size); this.data.addBackpack(backpack); + markDirty(); + return backpack; } @@ -260,6 +264,7 @@ public final void markDirty() { PlayerBackpack backpack = data.getBackpack(id); if (backpack != null) { + markDirty(); return Optional.of(backpack); } 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 535eabc9b9..5b35e5e4cc 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 @@ -76,6 +76,7 @@ public PlayerData loadPlayerData(@Nonnull UUID uuid) { return new PlayerData(researches, backpacks, waypoints); } + // The current design of saving all at once isn't great, this will be refined. @Override public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -83,6 +84,7 @@ public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); // Save research + playerFile.setValue("rearches", null); for (Research research : data.getResearches()) { // Legacy data uses IDs playerFile.setValue("researches." + research.getID(), true); @@ -96,11 +98,16 @@ public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { ItemStack item = backpack.getInventory().getItem(i); if (item != null) { playerFile.setValue("backpacks." + backpack.getId() + ".contents." + i, item); + + // Remove the item if it's no longer in the inventory + } else if (playerFile.contains("backpacks." + backpack.getId() + ".contents." + i)) { + playerFile.setValue("backpacks." + backpack.getId() + ".contents." + i, null); } } } // Save waypoints + waypointsFile.clear(); for (Waypoint waypoint : data.getWaypoints()) { // Legacy data uses IDs waypointsFile.setValue(waypoint.getId(), waypoint.getLocation()); From d29124ee5e916d6ecb63bbfe61e68f834ed51212 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 20 Dec 2023 21:41:05 +0000 Subject: [PATCH 11/14] Documentation around deprecated --- .../thebusybiscuit/slimefun4/api/gps/Waypoint.java | 4 ++-- .../slimefun4/api/player/PlayerBackpack.java | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java index b7d8580c70..3286608049 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java @@ -63,9 +63,9 @@ public Waypoint(UUID ownerId, String id, Location loc, String name) { } /** - * This returns the owner {@link UUID} of the {@link Waypoint}. + * This returns the owner's {@link UUID} of the {@link Waypoint}. * - * @return The corresponding owner {@link UUID} + * @return The corresponding owner's {@link UUID} */ @Nonnull public UUID getOwnerId() { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java index 593c08852f..c0886778df 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java @@ -60,6 +60,8 @@ private PlayerBackpack(@Nonnull UUID ownerId, int id, int size) { * The {@link PlayerProfile} of this Backpack * @param id * The id of this Backpack + * + * @deprecated Use {@link PlayerBackpack#load(UUID, int, int, HashMap)} instead */ @Deprecated public PlayerBackpack(@Nonnull PlayerProfile profile, int id) { @@ -79,6 +81,8 @@ public PlayerBackpack(@Nonnull PlayerProfile profile, int id) { * The id of this Backpack * @param size * The size of this Backpack + * + * @deprecated Use {@link PlayerBackpack#newBackpack(UUID, int, int)} instead */ @Deprecated public PlayerBackpack(@Nonnull PlayerProfile profile, int id, int size) { @@ -111,6 +115,8 @@ public int getId() { * This method returns the {@link PlayerProfile} this {@link PlayerBackpack} belongs to * * @return The owning {@link PlayerProfile} + * + * @deprecated Use {@link PlayerBackpack#getOwnerId()} instead */ @Deprecated @Nonnull @@ -206,6 +212,8 @@ public void setSize(int size) { /** * This method will save the contents of this backpack to a {@link File}. + * + * @deprecated Handled by {@link PlayerProfile#save()} now */ @Deprecated public void save() { @@ -218,7 +226,6 @@ public void save() { * This method marks the backpack dirty, it will then be queued for an autosave * using {@link PlayerBackpack#save()} */ - @Deprecated public void markDirty() { if (profile != null) { profile.markDirty(); @@ -235,7 +242,6 @@ private void setContents(int size, HashMap contents) { } } - // NTS: The data loading is handled in the storage layer but we handle this specific class here @ParametersAreNonnullByDefault public static PlayerBackpack load(UUID ownerId, int id, int size, HashMap contents) { PlayerBackpack backpack = new PlayerBackpack(ownerId, id, size); From 6ffd63d6b3416dd2b937e098b45dc75c8be28480 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 20 Dec 2023 23:53:18 +0000 Subject: [PATCH 12/14] Add some more tests --- .../storage/backend/legacy/LegacyStorage.java | 12 ++- .../storage/backend/TestLegacyBackend.java | 97 +++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) 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 5b35e5e4cc..d7981a5466 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 @@ -85,9 +85,15 @@ public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { // Save research playerFile.setValue("rearches", null); - for (Research research : data.getResearches()) { - // Legacy data uses IDs - playerFile.setValue("researches." + research.getID(), true); + for (Research research : Slimefun.getRegistry().getResearches()) { + // Save the research if it's researched + if (data.getResearches().contains(research)) { + playerFile.setValue("researches." + research.getID(), true); + + // Remove the research if it's no longer researched + } else if (playerFile.contains("researches." + research.getID())) { + playerFile.setValue("researches." + research.getID(), null); + } } // Save backpacks 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 804f24306a..1859659999 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 @@ -62,6 +62,7 @@ public static void unload() throws IOException { FileUtils.deleteDirectory(new File("data-storage")); } + // Test simple loading and saving of player data @Test void testLoadingResearches() throws IOException { // Create a player file which we can load @@ -275,6 +276,102 @@ void testSavingWaypoints() throws InterruptedException { Assertions.assertEquals("world", waypoint.getLocation().getWorld().getName()); } + // Test realistic situations + @Test + void testResearchChanges() throws InterruptedException { + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + // Unlock all researches + for (Research research : Slimefun.getRegistry().getResearches()) { + profile.setResearched(research, 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); + Assertions.assertEquals(10, assertion.getResearches().size()); + for (int i = 0; i < 10; i++) { + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); + } + + // Now let's change the data and save it again + profile.setResearched(Slimefun.getRegistry().getResearches().get(3), false); + + // Save the player data + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + System.out.println("update assertion"); + assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(9, assertion.getResearches().size()); + for (int i = 0; i < 10; i++) { + if (i != 3) { + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); + } + } + } + + // Test realistic situations - when we fix the serialization issue + // @Test + // void testBackpackChanges() throws InterruptedException {} + + @Test + void testWaypointChanges() throws InterruptedException { + // Create mock world + World world = server.createWorld(WorldCreator.name("world").environment(Environment.NORMAL)); + + // 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); + + profile.addWaypoint(new Waypoint( + player.getUniqueId(), + "test", + new Location(world, 1, 2, 3, 4, 5), + ChatColor.GREEN + "Test waypoint" + )); + + Waypoint test2 = new Waypoint( + player.getUniqueId(), + "test2", + new Location(world, 10, 20, 30, 40, 50), + ChatColor.GREEN + "Test 2 waypoint" + ); + profile.addWaypoint(test2); + + // 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); + Assertions.assertEquals(2, assertion.getWaypoints().size()); + + // Remove one + profile.removeWaypoint(test2); + + // Save the player data + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(1, assertion.getWaypoints().size()); + } + // Utils private static void setupResearches() { for (int i = 0; i < 10; i++) { From 12a7f55d9ff4e379b54bd80f75b53da7f7239320 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 27 Dec 2023 03:03:27 +0000 Subject: [PATCH 13/14] Some small doc updates --- docs/adr/0001-storage-layer.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md index 1d04549295..0809ed04a2 100644 --- a/docs/adr/0001-storage-layer.md +++ b/docs/adr/0001-storage-layer.md @@ -1,12 +1,13 @@ # 1. Storage layer Date: 2023-11-15 +Last update: 2023-12-27 **DO NOT rely on any APIs introduced until we finish the work completely!** ## Status -Proposed +Work in progress ## Context @@ -52,7 +53,7 @@ as possible. ### Implementation details -There is a new interface called [`Storage`]() which is what all storage +There is a new interface called [`Storage`](TBD) which is what all storage backends will implement. This will have methods for loading and saving things like [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java) and [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). @@ -81,6 +82,8 @@ possible. Changes will be tested while in the PR stage and merged into the Dev releases when possible. We may do an experimental release if required. +Phases do not (and very likely will not) be done within a single PR. They will also not have any timeframe attached to them. + The current plan looks like this: * Phase 1 - Implement legacy data backend for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). @@ -97,7 +100,7 @@ The current plan looks like this: * Mark it as stable and remove the warnings once we're sure things are working correctly * Create a migration path for users currently using "legacy". - * **MAYBE** enable by default for new servers? + * Enable by default for new servers * Phase 4 - Move [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java) to new storage layer. * The big one! We're gonna tackle adding this to BlockStorage. This will probably be a large change and we'll want to be as @@ -109,7 +112,7 @@ The current plan looks like this: * Mark it as stable and remove the warnings once we're sure things are working correctly * Ensure migration path works here too. - * **MAYBE** enable by default for new servers? + * Enable by default for new servers * Phase 6 - Finish up and move anything else we want over * Move over any other data stores we have to the new layer * We should probably still do experimental -> stable but it should have @@ -118,3 +121,9 @@ The current plan looks like this: ## State of work * Phase 1: In progress + * https://github.com/Slimefun/Slimefun4/pull/4065 +* Phase 2: Not started +* Phase 3: Not started +* Phase 4: Not started +* Phase 5: Not started +* Phase 6: Not started From 33fae27fadf933a8bd43990a48a2e6e84fde27fa Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Tue, 2 Jan 2024 02:03:29 +0000 Subject: [PATCH 14/14] Make old Waypoint constructor deprecated and fix javadocs --- .../slimefun4/api/gps/Waypoint.java | 22 ++++++++++++++++++- .../slimefun4/api/items/SlimefunItem.java | 8 +++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java index 3286608049..468b70af6b 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java @@ -37,6 +37,26 @@ public class Waypoint { private final String name; private final Location location; + /** + * This constructs a new {@link Waypoint} object. + * + * @param profile + * The owning {@link PlayerProfile} + * @param id + * The unique id for this {@link Waypoint} + * @param loc + * The {@link Location} of the {@link Waypoint} + * @param name + * The name of this {@link Waypoint} + * + * @deprecated Use {@link #Waypoint(UUID, String, Location, String)} instead + */ + @Deprecated + @ParametersAreNonnullByDefault + public Waypoint(PlayerProfile profile, String id, Location loc, String name) { + this(profile.getUUID(), id, loc, name); + } + /** * This constructs a new {@link Waypoint} object. * @@ -77,7 +97,7 @@ public UUID getOwnerId() { * * @return The corresponding {@link PlayerProfile} * - * @deprecated Use {@link #getUuid()} instead + * @deprecated Use {@link #getOwnerId()} instead */ @Nonnull @Deprecated diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java index 20a12ef6b7..28dc680ff1 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java @@ -1160,11 +1160,11 @@ public final int hashCode() { } /** - * Retrieve a {@link Optional}<{@link SlimefunItem}> by its id. + * Retrieve a {@link Optional} {@link SlimefunItem} by its id. * * @param id * The id of the {@link SlimefunItem} - * @return The {@link Optional}<{@link SlimefunItem}> associated with that id. Empty if non-existent + * @return The {@link Optional} {@link SlimefunItem} associated with that id. Empty if non-existent */ public static @Nonnull Optional getOptionalById(@Nonnull String id) { return Optional.ofNullable(getById(id)); @@ -1193,11 +1193,11 @@ public final int hashCode() { } /** - * Retrieve a {@link Optional}<{@link SlimefunItem}> from an {@link ItemStack}. + * Retrieve a {@link Optional} {@link SlimefunItem} from an {@link ItemStack}. * * @param item * The {@link ItemStack} to check - * @return The {@link Optional}<{@link SlimefunItem}> associated with this {@link ItemStack} if present, otherwise empty + * @return The {@link Optional} {@link SlimefunItem} associated with this {@link ItemStack} if present, otherwise empty */ public @Nonnull Optional getOptionalByItem(@Nullable ItemStack item) { return Optional.ofNullable(getByItem(item));