Fix memory leak with loaded players' advancements

internal/v1_18_R2/src/main/java/com/lishid/openinv/internal/v1_18_R2/PlayerDataManager.java

@@ -99,6 +99,9 @@ public PlayerDataManager() {
 
         ServerPlayer entity = new ServerPlayer(server, worldServer, profile);
 
+        // Stop listening for advancement progression - if this is not cleaned up, loading causes a memory leak.
+        entity.getAdvancements().stopListening();
+
         try {
             injectPlayer(entity);
         } catch (IllegalAccessException e) {

internal/v1_18_R2/src/main/java/com/lishid/openinv/internal/v1_18_R2/SpecialEnderChest.java

@@ -67,18 +67,29 @@ public void setPlayerOffline() {
 
     @Override
     public void setPlayerOnline(@NotNull final org.bukkit.entity.Player player) {
-        if (!this.playerOnline) {
-            try {
-                this.owner = PlayerDataManager.getHandle(player);
-                PlayerEnderChestContainer enderChest = owner.getEnderChestInventory();
-                for (int i = 0; i < enderChest.getContainerSize(); ++i) {
-                    enderChest.setItem(i, this.items.get(i));
-                }
-                this.items = enderChest.items;
-                enderChest.transaction.addAll(this.transaction);
-            } catch (Exception ignored) {}
-            this.playerOnline = true;
+        if (this.playerOnline) {
+            return;
+        }
+
+        ServerPlayer offlinePlayer = this.owner;
+        ServerPlayer onlinePlayer = PlayerDataManager.getHandle(player);
+
+        // Set owner to new player.
+        this.owner = onlinePlayer;
+
+        // Set player's ender chest contents to our modified contents.
+        PlayerEnderChestContainer onlineEnderChest = onlinePlayer.getEnderChestInventory();
+        for (int i = 0; i < onlineEnderChest.getContainerSize(); ++i) {
+            onlineEnderChest.setItem(i, this.items.get(i));
         }
+
+        // Set our item array to the new inventory's array.
+        this.items = onlineEnderChest.items;
+
+        // Add viewers to new inventory.
+        onlineEnderChest.transaction.addAll(offlinePlayer.getEnderChestInventory().transaction);
+
+        this.playerOnline = true;
     }
 
     @Override
@@ -317,7 +328,7 @@ public ItemStack removeItemType(Item item, int i) {
 
     @Override
     public String toString() {
-        return this.items.stream().filter((itemStack) -> !itemStack.isEmpty()).collect(Collectors.toList()).toString();
+        return this.items.stream().filter((itemStack) -> !itemStack.isEmpty()).toList().toString();
     }
 
     @Override

internal/v1_18_R2/src/main/java/com/lishid/openinv/internal/v1_18_R2/SpecialPlayerInventory.java

@@ -77,20 +77,37 @@ public SpecialPlayerInventory(@NotNull org.bukkit.entity.Player bukkitPlayer, @N
 
     @Override
     public void setPlayerOnline(@NotNull org.bukkit.entity.Player player) {
-        if (!this.playerOnline) {
-            Player entityPlayer = PlayerDataManager.getHandle(player);
-            entityPlayer.getInventory().transaction.addAll(this.transaction);
-            this.player = entityPlayer;
-            for (int i = 0; i < getContainerSize(); ++i) {
-                this.player.getInventory().setItem(i, getRawItem(i));
-            }
-            this.player.getInventory().selected = this.selected;
-            this.items = this.player.getInventory().items;
-            this.armor = this.player.getInventory().armor;
-            this.offhand = this.player.getInventory().offhand;
-            this.compartments = ImmutableList.of(this.items, this.armor, this.offhand);
-            this.playerOnline = true;
+        if (this.playerOnline) {
+            return;
+        }
+
+        Player offlinePlayer = this.player;
+        Player onlinePlayer = PlayerDataManager.getHandle(player);
+        onlinePlayer.getInventory().transaction.addAll(this.transaction);
+
+        // Set owner to new player.
+        this.player = onlinePlayer;
+
+        // Set player's inventory contents to our modified contents.
+        Inventory onlineInventory = onlinePlayer.getInventory();
+        for (int i = 0; i < getContainerSize(); ++i) {
+            onlineInventory.setItem(i, getRawItem(i));
         }
+        onlineInventory.selected = this.selected;
+
+        // Set our item arrays to the new inventory's arrays.
+        this.items = onlineInventory.items;
+        this.armor = onlineInventory.armor;
+        this.offhand = onlineInventory.offhand;
+        this.compartments = ImmutableList.of(this.items, this.armor, this.offhand);
+
+        // Add existing viewers to new viewer list.
+        Inventory offlineInventory = offlinePlayer.getInventory();
+        // Remove self from listing - player is always a viewer of their own inventory, prevent duplicates.
+        offlineInventory.transaction.remove(offlinePlayer.getBukkitEntity());
+        onlineInventory.transaction.addAll(offlineInventory.transaction);
+
+        this.playerOnline = true;
     }
 
     @Override
@@ -138,7 +155,7 @@ private void setRawItem(int i, @NotNull ItemStack itemStack) {
         }
     }
 
-    private static record IndexedCompartment(@Nullable NonNullList<ItemStack> compartment, int index) {}
+    private record IndexedCompartment(@Nullable NonNullList<ItemStack> compartment, int index) {}
 
     private @NotNull SpecialPlayerInventory.IndexedCompartment getIndexedContent(int index) {
         if (index < items.size()) {

internal/v1_19_R1/src/main/java/com/lishid/openinv/internal/v1_19_R1/PlayerDataManager.java

@@ -100,6 +100,9 @@ public PlayerDataManager() {
 
         ServerPlayer entity = new ServerPlayer(server, worldServer, profile, null);
 
+        // Stop listening for advancement progression - if this is not cleaned up, loading causes a memory leak.
+        entity.getAdvancements().stopListening();
+
         try {
             injectPlayer(entity);
         } catch (IllegalAccessException e) {

internal/v1_19_R1/src/main/java/com/lishid/openinv/internal/v1_19_R1/SpecialEnderChest.java

@@ -67,18 +67,29 @@ public void setPlayerOffline() {
 
     @Override
     public void setPlayerOnline(@NotNull final org.bukkit.entity.Player player) {
-        if (!this.playerOnline) {
-            try {
-                this.owner = PlayerDataManager.getHandle(player);
-                PlayerEnderChestContainer enderChest = owner.getEnderChestInventory();
-                for (int i = 0; i < enderChest.getContainerSize(); ++i) {
-                    enderChest.setItem(i, this.items.get(i));
-                }
-                this.items = enderChest.items;
-                enderChest.transaction.addAll(this.transaction);
-            } catch (Exception ignored) {}
-            this.playerOnline = true;
+        if (this.playerOnline) {
+            return;
+        }
+
+        ServerPlayer offlinePlayer = this.owner;
+        ServerPlayer onlinePlayer = PlayerDataManager.getHandle(player);
+
+        // Set owner to new player.
+        this.owner = onlinePlayer;
+
+        // Set player's ender chest contents to our modified contents.
+        PlayerEnderChestContainer onlineEnderChest = onlinePlayer.getEnderChestInventory();
+        for (int i = 0; i < onlineEnderChest.getContainerSize(); ++i) {
+            onlineEnderChest.setItem(i, this.items.get(i));
         }
+
+        // Set our item array to the new inventory's array.
+        this.items = onlineEnderChest.items;
+
+        // Add viewers to new inventory.
+        onlineEnderChest.transaction.addAll(offlinePlayer.getEnderChestInventory().transaction);
+
+        this.playerOnline = true;
     }
 
     @Override

internal/v1_19_R1/src/main/java/com/lishid/openinv/internal/v1_19_R1/SpecialPlayerInventory.java

@@ -77,20 +77,37 @@ public SpecialPlayerInventory(@NotNull org.bukkit.entity.Player bukkitPlayer, @N
 
     @Override
     public void setPlayerOnline(@NotNull org.bukkit.entity.Player player) {
-        if (!this.playerOnline) {
-            Player entityPlayer = PlayerDataManager.getHandle(player);
-            entityPlayer.getInventory().transaction.addAll(this.transaction);
-            this.player = entityPlayer;
-            for (int i = 0; i < getContainerSize(); ++i) {
-                this.player.getInventory().setItem(i, getRawItem(i));
-            }
-            this.player.getInventory().selected = this.selected;
-            this.items = this.player.getInventory().items;
-            this.armor = this.player.getInventory().armor;
-            this.offhand = this.player.getInventory().offhand;
-            this.compartments = ImmutableList.of(this.items, this.armor, this.offhand);
-            this.playerOnline = true;
+        if (this.playerOnline) {
+            return;
+        }
+
+        Player offlinePlayer = this.player;
+        Player onlinePlayer = PlayerDataManager.getHandle(player);
+        onlinePlayer.getInventory().transaction.addAll(this.transaction);
+
+        // Set owner to new player.
+        this.player = onlinePlayer;
+
+        // Set player's inventory contents to our modified contents.
+        Inventory onlineInventory = onlinePlayer.getInventory();
+        for (int i = 0; i < getContainerSize(); ++i) {
+            onlineInventory.setItem(i, getRawItem(i));
         }
+        onlineInventory.selected = this.selected;
+
+        // Set our item arrays to the new inventory's arrays.
+        this.items = onlineInventory.items;
+        this.armor = onlineInventory.armor;
+        this.offhand = onlineInventory.offhand;
+        this.compartments = ImmutableList.of(this.items, this.armor, this.offhand);
+
+        // Add existing viewers to new viewer list.
+        Inventory offlineInventory = offlinePlayer.getInventory();
+        // Remove self from listing - player is always a viewer of their own inventory, prevent duplicates.
+        offlineInventory.transaction.remove(offlinePlayer.getBukkitEntity());
+        onlineInventory.transaction.addAll(offlineInventory.transaction);
+
+        this.playerOnline = true;
     }
 
     @Override

plugin/src/main/java/com/lishid/openinv/OpenInv.java

@@ -476,8 +476,9 @@ void handleCloseInventory(@NotNull HumanEntity exViewer, @NotNull ISpecialInvent
 
             if (!disableSaving()
                     && current != null
-                    && current.getPlayer() instanceof Player player && !player.isOnline()) {
-                this.accessor.getPlayerDataManager().inject(player).saveData();
+                    && current.getPlayer() instanceof Player player
+                    && !player.isOnline()) {
+                    this.accessor.getPlayerDataManager().inject(player).saveData();
             }
         });
     }

plugin/src/main/java/com/lishid/openinv/internal/IPlayerDataManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011-2021 lishid. All rights reserved.
+ * Copyright (C) 2011-2022 lishid. All rights reserved.
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by

plugin/src/main/java/com/lishid/openinv/internal/OpenInventoryView.java

@@ -27,13 +27,17 @@
 
 public class OpenInventoryView extends InventoryView {
 
-    private final Player player;
-    private final ISpecialInventory inventory;
-    private final String titleKey;
-    private final String titleDefaultSuffix;
+    private final @NotNull Player player;
+    private final @NotNull ISpecialInventory inventory;
+    private final @NotNull String titleKey;
+    private final @NotNull String titleDefaultSuffix;
     private String title;
 
-    public OpenInventoryView(Player player, ISpecialInventory inventory, String titleKey, String titleDefaultSuffix) {
+    public OpenInventoryView(
+            @NotNull Player player,
+            @NotNull ISpecialInventory inventory,
+            @NotNull String titleKey,
+            @NotNull String titleDefaultSuffix) {
         this.player = player;
         this.inventory = inventory;
         this.titleKey = titleKey;