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.

platforms/spigot/src/main/java/net/codersky/mcutils/spigot/SpigotUtils.java

@@ -4,14 +4,14 @@
 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;
 import net.codersky.mcutils.java.strings.MCStrings;
 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<P extends JavaPlugin> extends ServerUtils<P> {
 
 	private final SpigotConsole console;
-	private PlayerProvider<Player> 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<P> setPlayerProvider(@NotNull PlayerProvider<Player> playerProvider) {
+	public SpigotUtils<P> 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<Player> 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

platforms/spigot/src/main/java/net/codersky/mcutils/spigot/player/SpigotPlayerProvider.java

@@ -21,4 +21,9 @@ public class SpigotPlayerProvider extends PlayerProvider<Player> {
 	public @Nullable UUID getUUID(@NotNull Player handle) {
 		return handle.getUniqueId();
 	}
+
+	@Override
+	protected final void removeFromCache(@NotNull UUID uuid) {
+		super.removeFromCache(uuid);
+	}
 }

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());
+	}
+}

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<P> extends ProxyUtils<P> {
 
 	private final ProxyServer proxy;
 	private final VelocityConsole console;
-	private PlayerProvider<Player> 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<P> setPlayerProvider(@NotNull PlayerProvider<Player> playerProvider) {
+	public VelocityUtils<P> 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<Player> 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;

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);
+	}
 }

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());
+	}
+}

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<T> {
 	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);
+	}
 }