From d24827ffcb8725f5befc9abe1b92c5a3f3b8ac61 Mon Sep 17 00:00:00 2001 From: Jikoo Date: Wed, 14 Dec 2016 19:49:18 -0500 Subject: [PATCH] Allow plugins to indicate to OpenInv that a Player is in use This allows API users to prevent issues caused by multiple different copies of the Player being loaded, such as #49. Multiple instances of the same player could be obtained by calling IOpenInv#loadPlayer, waiting for OpenInv to remove it from the cache, then calling the method again. --- .../java/com/lishid/openinv/IOpenInv.java | 34 ++++++++++++++++ .../main/java/com/lishid/openinv/OpenInv.java | 39 ++++++++++++++++++- pom.xml | 2 +- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/com/lishid/openinv/IOpenInv.java b/api/src/main/java/com/lishid/openinv/IOpenInv.java index 28514fc..775a63b 100644 --- a/api/src/main/java/com/lishid/openinv/IOpenInv.java +++ b/api/src/main/java/com/lishid/openinv/IOpenInv.java @@ -7,6 +7,7 @@ import com.lishid.openinv.internal.ISpecialPlayerInventory; import org.bukkit.OfflinePlayer; import org.bukkit.entity.Player; +import org.bukkit.plugin.Plugin; /** * Interface defining behavior for the OpenInv plugin. @@ -150,4 +151,37 @@ public interface IOpenInv { */ public Player loadPlayer(final OfflinePlayer offline); + /** + * Mark a Player as in use by a Plugin to prevent it from being removed from the cache. Used to + * prevent issues with multiple copies of the same Player being loaded such as lishid#49. + * Changes made to loaded copies overwrite changes to the others when saved, leading to + * duplication bugs and more. + *

+ * When finished with the Player object, be sure to call {@link #releasePlayer(Player, Plugin)} + * to prevent the cache from keeping it stored until the plugin is disabled. + *

+ * When using a Player object from OpenInv, you must handle the Player coming online, replacing + * your Player reference with the Player from the PlayerJoinEvent. In addition, you must change + * any values in the Player to reflect any unsaved alterations to the existing Player which do + * not affect the inventory or ender chest contents. + *

+ * OpenInv only saves player data when unloading a Player from the cache, and then only if + * {@link #disableSaving()} returns false. If you are making changes that OpenInv does not cause + * to persist when a Player logs in as noted above, it is suggested that you manually call + * {@link Player#saveData()} when releasing your reference to ensure your changes persist. + * + * @param player the Player + * @param plugin the Plugin holding the reference to the Player + */ + public void retainPlayer(Player player, Plugin plugin); + + /** + * Mark a Player as no longer in use by a Plugin to allow OpenInv to remove it from the cache + * when eligible. + * + * @param player the Player + * @param plugin the Plugin no longer holding a reference to the Player + */ + public void releasePlayer(Player player, Plugin plugin); + } diff --git a/plugin/plugin-core/src/main/java/com/lishid/openinv/OpenInv.java b/plugin/plugin-core/src/main/java/com/lishid/openinv/OpenInv.java index 60f4733..334e5cc 100644 --- a/plugin/plugin-core/src/main/java/com/lishid/openinv/OpenInv.java +++ b/plugin/plugin-core/src/main/java/com/lishid/openinv/OpenInv.java @@ -44,12 +44,16 @@ import com.lishid.openinv.util.Function; import com.lishid.openinv.util.InternalAccessor; import com.lishid.openinv.util.Permissions; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Multimap; + import org.bukkit.Bukkit; import org.bukkit.OfflinePlayer; import org.bukkit.command.PluginCommand; import org.bukkit.entity.HumanEntity; import org.bukkit.entity.Player; import org.bukkit.inventory.Inventory; +import org.bukkit.plugin.Plugin; import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.java.JavaPlugin; import org.bukkit.scheduler.BukkitRunnable; @@ -63,13 +67,17 @@ public class OpenInv extends JavaPlugin implements IOpenInv { private final Map inventories = new HashMap(); private final Map enderChests = new HashMap(); + // TODO: handle plugin unload + private final Multimap> pluginUsage = HashMultimap.create(); + private final Cache playerCache = new Cache(300000L, new Function() { @Override public boolean run(Player value) { String key = playerLoader.getPlayerDataID(value); return inventories.containsKey(key) && inventories.get(key).isInUse() - || enderChests.containsKey(key) && enderChests.get(key).isInUse(); + || enderChests.containsKey(key) && enderChests.get(key).isInUse() + || pluginUsage.containsKey(key); } }, new Function() { @@ -427,6 +435,7 @@ public class OpenInv extends JavaPlugin implements IOpenInv { return this.playerCache.get(key); } + // TODO: wrap Player to ensure all methods can safely be called offline Player loaded; if (offline.isOnline()) { @@ -479,6 +488,34 @@ public class OpenInv extends JavaPlugin implements IOpenInv { return loaded; } + /** + * @see com.lishid.openinv.IOpenInv#retainPlayer(org.bukkit.entity.Player, org.bukkit.plugin.Plugin) + */ + @Override + public void retainPlayer(Player player, Plugin plugin) { + String key = this.playerLoader.getPlayerDataID(player); + + if (this.pluginUsage.containsEntry(key, plugin.getClass())) { + return; + } + + this.pluginUsage.put(key, plugin.getClass()); + } + + /** + * @see com.lishid.openinv.IOpenInv#releasePlayer(org.bukkit.entity.Player, org.bukkit.plugin.Plugin) + */ + @Override + public void releasePlayer(Player player, Plugin plugin) { + String key = this.playerLoader.getPlayerDataID(player); + + if (!this.pluginUsage.containsEntry(key, plugin.getClass())) { + return; + } + + this.pluginUsage.remove(key, plugin.getClass()); + } + /** * Method for handling a Player coming online. * diff --git a/pom.xml b/pom.xml index cc71f52..7dea94c 100644 --- a/pom.xml +++ b/pom.xml @@ -12,7 +12,7 @@ UTF-8 - 3.0.3-SNAPSHOT + 3.0.4-SNAPSHOT