Player close inventory

Discussion in 'Spigot Plugin Development' started by QuestPx, Jul 28, 2018.

  1. Hello everyone, why does this method give an error? opened - a list of players to which I add a player after the Inventory Inventor run and delete after running InventoryCloseEvent
    Code (Text):

    public void clearOpenedInventory() {
            for(Player p : opened) {
                if(p!=null && p.isOnline()) {
                p.closeInventory();
                }
            }
            opened.clear();
        }

     
     
  2. Is that all the code?
    What is the error?
     
  3. post the stack trace pls
     
  4. Code (Text):


    [14:59:16 ERROR]: Could not pass event InventoryClickEvent to vBattle v1.0
    org.bukkit.event.EventException: null
            at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:62) ~[spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:500) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:485) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.PlayerConnection.a(PlayerConnection.java:1889) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.PacketPlayInWindowClick.a(SourceFile:33) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.PacketPlayInWindowClick.a(SourceFile:10) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.PlayerConnectionUtils$1.run(SourceFile:13) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:1.8.0_181]
            at java.util.concurrent.FutureTask.run(FutureTask.java:266) [?:1.8.0_181]
            at net.minecraft.server.v1_12_R1.SystemUtils.a(SourceFile:46) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.MinecraftServer.D(MinecraftServer.java:748) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.DedicatedServer.D(DedicatedServer.java:406) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.MinecraftServer.C(MinecraftServer.java:679) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at net.minecraft.server.v1_12_R1.MinecraftServer.run(MinecraftServer.java:577) [spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            at java.lang.Thread.run(Thread.java:748) [?:1.8.0_181]
    Caused by: java.util.ConcurrentModificationException
            at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:909) ~[?:1.8.0_181]
            at java.util.ArrayList$Itr.next(ArrayList.java:859) ~[?:1.8.0_181]
            at net.questpx.battle.clearOpenedInventory(Battle.java:73) ~[?:?]
            at net.questpx.battle.Listeners.onInventoryInteract(Listeners.java:48) ~[?:?]
            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_181]
            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_181]
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_181]
            at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_181]
            at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:302) ~[spigot-1.12.2.jar:git-Spigot-3d850ec-809c399]
            ... 15 more
     
  5. Code (Text):
    Caused by: java.util.ConcurrentModificationException
    <<< ERROR

    You are trying to modify a list or array list you are looping in. I can not say what loop it is because you did not provide the whole methods that access that method

    I guess its this

    Code (Text):
    net.questpx.battle.clearOpenedInventory(Case.java:73) ~[?:?]
     
  6. I add one player to the list (opened), then I call a method that should remove this player from the list (opened) and close his inventory
     
  7. 73: for(Player p : opened) {
     
  8. Thats not the code :D thats what the code is meant to be doing :)
     
  9. Go ahead and show where you create your arraylist (or however you're storing the data) and all the methods that add to or remove from the opened list.
    Too many things are going on at once in your collection, or you have a paradox somewhere.
     
  10. List<Player> opened = new ArrayList<Player>();
     
  11. I am going to guess here

    Your "opened" ArrayList reference is null
    Check if some code is nulling this reference or if you have initialized this properly.
     
  12. tried to check opened != zero and opened.size() != 0
     
  13. Okay.
    Here's your problem. You haven't posted the code, but when you close your player's inventory, I'm guessing you have some rule in place that removes them from the open list. Arraylists don't like that. They don't want you to remove values while you're iterating over them without directly removing them.
    So try explicitly removing them from the list in that method before you close their inventory. And if you have some rule in place that constantly removes players from the opened list if their inventory isn't opened, get rid of that.
     
  14. I see the problem now you are modifying the player variable inside a loop. This is your problem.

    Now try this do not save the whole player in the ArrayList USE PLAYER UUID
    Store the player UUID in a list instead of the Player

    Code (Text):
    List<UUID> openedInvPlayer = new ArrayList<>();

     
  15. use CopyOnWriteArrayList or synchronized, and use uuid instead of player
     
  16. There's absolutely no harm to storing Player instances in a Collection. Using an UUID instance or a Player instance are both doable and acceptable. The only difference is that the UUID instance will take less memory space as far as I'm aware of. Correct me on that if I'm wrong.
     
  17. yes, but he can modify the player from the collection, and what is better solution: looping players in collection and find player with the same uuid, or just get the uuid and use Bukkit.getPlayer or something like that (i do not rely on this code /\)
     
  18. Isn't this the same by doing Player#getUniqueId? Why using the Bukkit#getPlayer makes it better than using the Player instance?
     
  19. Yes there is no harm if you know what you are doing. The problem the OP is having is. He was trying to modify Player instances in a list which he was currently looping on. Which java will not permit as java will have to cancel the loop recalculate the new modified list values then start the loop again which will cause inconsistencies.

    Another example of code that will cause the same error

    Code (Text):
    import java.util.ArrayList;
    import java.util.List;
    import java.util.UUID;

    public class ExampleClass {
        private List<UUID> openedInvList = new ArrayList<>();
        public void filterPlayer(UUID pUUID) {
            for (UUID uuidBuffer : openedInvList) {
                if (uuidBuffer.equals(pUUID)) {
                    this.openedInvList.remove(uuidBuffer);
                }
            }
        }
    }
     
    If you were just trying to verify if an instance of player exist on the list it will be fine. But if you are trying to modify the instance of player in the list it will cause the error.

    To avoid this we use Player UUID instead

    Code (Text):
    import java.util.ArrayList;
    import java.util.List;
    import java.util.UUID;

    import org.bukkit.Bukkit;
    import org.bukkit.entity.Player;

    public class ExampleClass {
        private List<UUID> openedInvList = new ArrayList<>();
       
        public void filterPlayer() {
            for (Player player : Bukkit.getServer().getOnlinePlayers()) {
             
                if (openedInvList.contains(player.getUniqueId())) {
                    player.closeInventory();
                }
               
            }
         
            this.openedInvList.clear();
        }
    }