From 585aba929fe8f416ee37b85c75a894b18bb5ae2b Mon Sep 17 00:00:00 2001 From: xDec0de_ Date: Tue, 5 Nov 2024 12:11:16 +0100 Subject: [PATCH] Added listeners to clean PlayerProvider cache Alright so this is needed because PlayerProviders kept MCPlayer instances on cache indefinitely. This has been solved by adding a platform specific listener that removes MCPlayers fron the PlayerProvider cache whenever the Player leaves the server or proxy. I ended up using a "hacky" way to access the player cache by using protected methods on both SpigotPlayerProvider and VelocityPlayerProvider that call the super PlayerProvider method. This has been done so end users don't mess with the cache unintentionally, as they can only access this method by creating their own PlayerProvider. --- .../codersky/mcutils/spigot/SpigotUtils.java | 73 +++++++++++++++++-- .../spigot/player/SpigotPlayerProvider.java | 5 ++ .../player/SpigotPlayerQuitListener.java | 22 ++++++ .../mcutils/velocity/VelocityUtils.java | 70 +++++++++++++++--- .../player/VelocityPlayerProvider.java | 5 ++ .../player/VelocityPlayerQuitListener.java | 21 ++++++ .../crossplatform/player/PlayerProvider.java | 8 +- 7 files changed, 189 insertions(+), 15 deletions(-) create mode 100644 platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerQuitListener.java create mode 100644 platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerQuitListener.java diff --git a/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/SpigotUtils.java b/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/SpigotUtils.java index 6f8baba..6043913 100644 --- a/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/SpigotUtils.java +++ b/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/SpigotUtils.java @@ -4,7 +4,6 @@ import net.codersky.mcutils.cmd.GlobalCommand; import net.codersky.mcutils.cmd.MCCommand; import net.codersky.mcutils.crossplatform.player.MCPlayer; -import net.codersky.mcutils.crossplatform.player.PlayerProvider; import net.codersky.mcutils.crossplatform.server.ServerUtils; import net.codersky.mcutils.java.MCCollections; import net.codersky.mcutils.java.reflection.RefObject; @@ -12,6 +11,7 @@ import net.codersky.mcutils.spigot.cmd.AdaptedSpigotCommand; import net.codersky.mcutils.spigot.cmd.SpigotCommand; import net.codersky.mcutils.spigot.player.SpigotPlayerProvider; +import net.codersky.mcutils.spigot.player.SpigotPlayerQuitListener; import net.codersky.mcutils.spigot.worldgen.SingleBiomeProvider; import net.codersky.mcutils.spigot.worldgen.VoidGenerator; import org.bukkit.Bukkit; @@ -25,6 +25,7 @@ import org.bukkit.event.Listener; import org.bukkit.generator.BiomeProvider; import org.bukkit.generator.ChunkGenerator; +import org.bukkit.plugin.IllegalPluginAccessException; import org.bukkit.plugin.java.JavaPlugin; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -40,7 +41,8 @@ public class SpigotUtils

extends ServerUtils

{ private final SpigotConsole console; - private PlayerProvider playerProvider = new SpigotPlayerProvider(); + private SpigotPlayerProvider playerProvider = new SpigotPlayerProvider(); + private boolean isPlayerListenerOn = false; public SpigotUtils(@NotNull P plugin) { super(plugin); @@ -54,20 +56,81 @@ public File getDataFolder() { } @NotNull - public SpigotUtils

setPlayerProvider(@NotNull PlayerProvider playerProvider) { + public SpigotUtils

setPlayerProvider(@NotNull SpigotPlayerProvider playerProvider) { this.playerProvider = Objects.requireNonNull(playerProvider, "Player provider cannot be null"); return this; } + /** + * Gets the {@link SpigotPlayerProvider} being used by this {@link SpigotUtils} instance. + * + * @return The {@link SpigotPlayerProvider} being used by this {@link SpigotUtils} instance. + * + * @throws IllegalPluginAccessException If called before the plugin attempts to enable. + * This is because the very first time this method is called the {@link SpigotPlayerQuitListener} + * will be registered. This is required just to clean players that quit the server from the cache. + * + * @since MCUtils 1.0.0 + * + * @see #setPlayerProvider(SpigotPlayerProvider) + * @see #getPlayer(UUID) + * @see #getPlayer(Player) + */ @NotNull - public PlayerProvider getPlayerProvider() { + public SpigotPlayerProvider getPlayerProvider() { + if (!isPlayerListenerOn) { + Bukkit.getPluginManager().registerEvents(new SpigotPlayerQuitListener(this), getPlugin()); + isPlayerListenerOn = true; + } return playerProvider; } + /** + * Gets an {@link MCPlayer} by {@link UUID} from the + * {@link #getPlayerProvider() SpigotPlayerProvider} + * that this {@link SpigotUtils} is using. + * + * @param uuid The {@link UUID} of the player to get. + * + * @throws IllegalPluginAccessException If called before the plugin + * attempts to enable, see {@link #getPlayerProvider()} for details. + * + * @return A possibly {@code null} {@link MCPlayer} instance of an online + * {@link Player} that matches the provided {@link UUID}. + * + * @since MCUtils 1.0.0 + * + * @see #getPlayerProvider() + * @see #getPlayer(Player) + */ @Nullable @Override public MCPlayer getPlayer(@NotNull UUID uuid) { - return playerProvider.getPlayer(uuid); + return getPlayerProvider().getPlayer(uuid); + } + + /** + * Gets an {@link MCPlayer} by {@link Player} from the + * {@link #getPlayerProvider() SpigotPlayerProvider} + * that this {@link SpigotUtils} is using. + * + * @param bukkit The {@link Player} instance to convert. + * + * @throws IllegalPluginAccessException If called before the plugin + * attempts to enable, see {@link #getPlayerProvider()} for details. + * + * @return A {@link MCPlayer} instance that matches the provided {@link Player}. + * This can be {@code null} if you use an instance of a {@link Player} that + * is not {@link Player#isOnline() online}. + * + * @since MCUtils 1.0.0 + * + * @see #getPlayerProvider() + * @see #getPlayer(Player) + */ + @Nullable + public MCPlayer getPlayer(@NotNull Player bukkit) { + return getPlayerProvider().getPlayer(bukkit); } @NotNull diff --git a/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerProvider.java b/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerProvider.java index 97e7376..47e232e 100644 --- a/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerProvider.java +++ b/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerProvider.java @@ -21,4 +21,9 @@ public class SpigotPlayerProvider extends PlayerProvider { public @Nullable UUID getUUID(@NotNull Player handle) { return handle.getUniqueId(); } + + @Override + protected final void removeFromCache(@NotNull UUID uuid) { + super.removeFromCache(uuid); + } } diff --git a/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerQuitListener.java b/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerQuitListener.java new file mode 100644 index 0000000..9a50f55 --- /dev/null +++ b/platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerQuitListener.java @@ -0,0 +1,22 @@ +package net.codersky.mcutils.spigot.player; + +import net.codersky.mcutils.spigot.SpigotUtils; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerQuitEvent; +import org.jetbrains.annotations.NotNull; + +public class SpigotPlayerQuitListener implements Listener { + + private final SpigotUtils utils; + + public SpigotPlayerQuitListener(@NotNull SpigotUtils utils) { + this.utils = utils; + } + + @EventHandler(priority = EventPriority.HIGHEST) + public void onQuit(PlayerQuitEvent e) { + utils.getPlayerProvider().removeFromCache(e.getPlayer().getUniqueId()); + } +} diff --git a/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/VelocityUtils.java b/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/VelocityUtils.java index e808342..81703af 100644 --- a/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/VelocityUtils.java +++ b/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/VelocityUtils.java @@ -7,12 +7,12 @@ import net.codersky.mcutils.MCPlatform; import net.codersky.mcutils.cmd.GlobalCommand; import net.codersky.mcutils.crossplatform.player.MCPlayer; -import net.codersky.mcutils.crossplatform.player.PlayerProvider; import net.codersky.mcutils.crossplatform.proxy.ProxyUtils; import net.codersky.mcutils.java.MCCollections; import net.codersky.mcutils.velocity.cmd.AdaptedVelocityCommand; import net.codersky.mcutils.velocity.cmd.VelocityCommand; import net.codersky.mcutils.velocity.player.VelocityPlayerProvider; +import net.codersky.mcutils.velocity.player.VelocityPlayerQuitListener; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,8 +25,9 @@ public class VelocityUtils

extends ProxyUtils

{ private final ProxyServer proxy; private final VelocityConsole console; - private PlayerProvider playerProvider; + private VelocityPlayerProvider playerProvider; private final Path dataDirectory; + private boolean isPlayerListenerOn = false; public VelocityUtils(@NotNull P plugin, @NotNull ProxyServer proxy, @NotNull Path dataDirectory) { super(plugin); @@ -42,27 +43,78 @@ public final ProxyServer getProxy() { } @NotNull - public VelocityUtils

setPlayerProvider(@NotNull PlayerProvider playerProvider) { + public VelocityUtils

setPlayerProvider(@NotNull VelocityPlayerProvider playerProvider) { this.playerProvider = Objects.requireNonNull(playerProvider, "Player provider cannot be null"); return this; } + /** + * Gets the {@link VelocityPlayerProvider} being used by this {@link VelocityUtils} instance. + * + * @return The {@link VelocityPlayerProvider} being used by this {@link VelocityUtils} instance. + * + * @since MCUtils 1.0.0 + * + * @see #setPlayerProvider(VelocityPlayerProvider) + * @see #getPlayer(UUID) + * @see #getPlayer(Player) + */ @NotNull - public PlayerProvider getPlayerProvider() { + public VelocityPlayerProvider getPlayerProvider() { + if (!isPlayerListenerOn) { + proxy.getEventManager().register(getPlugin(), new VelocityPlayerQuitListener(this)); + isPlayerListenerOn = true; + } return playerProvider; } + /** + * Gets an {@link MCPlayer} by {@link UUID} from the + * {@link #getPlayerProvider() VelocityPlayerProvider} + * that this {@link VelocityUtils} is using. + * + * @param uuid The {@link UUID} of the player to get. + * + * @return A possibly {@code null} {@link MCPlayer} instance of an online + * {@link Player} that matches the provided {@link UUID}. + * + * @since MCUtils 1.0.0 + * + * @see #getPlayerProvider() + * @see #getPlayer(Player) + */ + @Nullable + public MCPlayer getPlayer(@NotNull UUID uuid) { + return getPlayerProvider().getPlayer(uuid); + } + + /** + * Gets an {@link MCPlayer} by {@link Player} from the + * {@link #getPlayerProvider() VelocityPlayerProvider} + * that this {@link VelocityUtils} is using. + * + * @param velocity The {@link Player} instance to convert. + * + * @return A {@link MCPlayer} instance that matches the provided {@link Player}. + * This can be {@code null} if you use an instance of a {@link Player} that + * is not {@link Player#isActive() online}. + * + * @since MCUtils 1.0.0 + * + * @see #getPlayerProvider() + * @see #getPlayer(Player) + */ + @Nullable + public MCPlayer getPlayer(@NotNull Player velocity) { + return getPlayerProvider().getPlayer(velocity); + } + @NotNull @Override public File getDataFolder() { return this.dataDirectory.toFile(); } - @Nullable - public MCPlayer getPlayer(@NotNull UUID uuid) { - return playerProvider.getPlayer(uuid); - } - @Override public @NotNull VelocityConsole getConsole() { return console; diff --git a/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerProvider.java b/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerProvider.java index ef6257f..3b8a234 100644 --- a/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerProvider.java +++ b/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerProvider.java @@ -30,4 +30,9 @@ protected MCPlayer fetchPlayer(@NotNull UUID uuid) { public UUID getUUID(@NotNull Player handle) { return handle.getUniqueId(); } + + @Override + protected final void removeFromCache(@NotNull UUID uuid) { + super.removeFromCache(uuid); + } } diff --git a/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerQuitListener.java b/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerQuitListener.java new file mode 100644 index 0000000..6b1f54d --- /dev/null +++ b/platforms/velocity/src/main/java/net/codersky/mcutils/velocity/player/VelocityPlayerQuitListener.java @@ -0,0 +1,21 @@ +package net.codersky.mcutils.velocity.player; + +import com.velocitypowered.api.event.PostOrder; +import com.velocitypowered.api.event.Subscribe; +import com.velocitypowered.api.event.connection.DisconnectEvent; +import net.codersky.mcutils.velocity.VelocityUtils; +import org.jetbrains.annotations.NotNull; + +public class VelocityPlayerQuitListener { + + private final VelocityUtils utils; + + public VelocityPlayerQuitListener(@NotNull VelocityUtils utils) { + this.utils = utils; + } + + @Subscribe(order = PostOrder.CUSTOM, priority = Short.MAX_VALUE) + public void onDisconnect(DisconnectEvent event) { + utils.getPlayerProvider().removeFromCache(event.getPlayer().getUniqueId()); + } +} diff --git a/shared/src/main/java/net/codersky/mcutils/crossplatform/player/PlayerProvider.java b/shared/src/main/java/net/codersky/mcutils/crossplatform/player/PlayerProvider.java index 27f8f3a..0a0c836 100644 --- a/shared/src/main/java/net/codersky/mcutils/crossplatform/player/PlayerProvider.java +++ b/shared/src/main/java/net/codersky/mcutils/crossplatform/player/PlayerProvider.java @@ -1,5 +1,6 @@ package net.codersky.mcutils.crossplatform.player; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -20,7 +21,7 @@ public abstract class PlayerProvider { public MCPlayer getPlayer(@NotNull UUID uuid) { MCPlayer player = playerCache.get(uuid); if (player != null) { - if (player.isOnline()) + if (player.isOnline()) // Player quit listeners should take care of this, but just in case... return player; playerCache.remove(uuid); } @@ -35,4 +36,9 @@ public MCPlayer getPlayer(@NotNull T original) { final UUID uuid = getUUID(original); return uuid == null ? null : getPlayer(uuid); } + + @ApiStatus.Internal + protected void removeFromCache(@NotNull UUID uuid) { + playerCache.remove(uuid); + } }