1.15.2 Removing Item from custom ArrayList<CustomItem>

Discussion in 'Spigot Plugin Development' started by arvenyon, Mar 6, 2020.

  1. Hey there,

    I am fairly new to java, so please go easy on me. Here's a short description of my situation:

    I have a custom class, which holds a player UUID, a slot index of his inventory and the ItemStack this slot contains:

    Code (Text):

    public class HeldItem {
        public UUID UniqueId;
        public int Slot;
        public ItemStack Item;
       
        public HeldItem(UUID UniqueId, int Slot, ItemStack Item) {
            this.UniqueId = UniqueId;
            this.Slot = Slot;
            this.Item = Item;
        }
    }
     
    A player can join and leave a state named "Skillbar-Mode".
    During the join process I fill an ArrayList<HeldItem> with the current ItemStacks the player has on specific hotbar slots:

    Code (Text):

    public static void joinSkillbarMode(Player Player) {
            for (int x : HotBarSlots) {
                if (Player.getInventory().getItem(x) != null) {
                    HeldItemList.add(new HeldItem(Player.getUniqueId(), x, Player.getInventory().getItem(x)));
                } else {
                    HeldItemList.add(new HeldItem(Player.getUniqueId(), x, new ItemStack(Material.AIR)));
                }
            }
            Player.getInventory().setHeldItemSlot(4);
        }
     
    When the player leaves the Skillbar-Mode, all previously held ItemStacks in those slots have to be restored, which works fine:

    Code (Text):

        public static void leaveSkillbarMode(Player Player) {
            for (HeldItem Found : HeldItemList) {
                if (Found.UniqueId.equals(Player.getUniqueId())) {
                    Player.getInventory().setItem(Found.Slot, Found.Item);
                    int x = HeldItemList.indexOf(Item);
                    HeldItemList.remove(x);
                }
            }
        }
     
    But 'simultaniously', the ArrayList should clear out the restored ItemStacks, since I wouldn't want it to fill up every time a player joins the Skillbar-Mode.
    Somehow thought, this doesen't work quite as well as I was hoping for, I get this error output:


    Code (Text):

    [20:16:36 ERROR]: null
    org.bukkit.command.CommandException: Unhandled exception executing command 'skillbar' in plugin arvenyonRPG v1.0
            at org.bukkit.command.PluginCommand.execute(PluginCommand.java:47) ~[patched_1.15.2.jar:git-Paper-98]
            at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:159) ~[patched_1.15.2.jar:git-Paper-98]
            at org.bukkit.craftbukkit.v1_15_R1.CraftServer.dispatchCommand(CraftServer.java:742) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.PlayerConnection.handleCommand(PlayerConnection.java:1821) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.PlayerConnection.a(PlayerConnection.java:1629) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.PacketPlayInChat.a(PacketPlayInChat.java:47) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.PacketPlayInChat.a(PacketPlayInChat.java:5) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.PlayerConnectionUtils.lambda$ensureMainThread$0(PlayerConnectionUtils.java:23) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.TickTask.run(SourceFile:18) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.IAsyncTaskHandler.executeTask(IAsyncTaskHandler.java:136) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.IAsyncTaskHandlerReentrant.executeTask(SourceFile:23) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.IAsyncTaskHandler.executeNext(IAsyncTaskHandler.java:109) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.MinecraftServer.ba(MinecraftServer.java:1038) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.MinecraftServer.executeNext(MinecraftServer.java:1031) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.IAsyncTaskHandler.executeAll(IAsyncTaskHandler.java:95) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.MinecraftServer.a(MinecraftServer.java:1172) ~[patched_1.15.2.jar:git-Paper-98]
            at net.minecraft.server.v1_15_R1.MinecraftServer.run(MinecraftServer.java:934) ~[patched_1.15.2.jar:git-Paper-98]
            at java.lang.Thread.run(Unknown Source) [?:1.8.0_231]
    Caused by: java.util.ConcurrentModificationException
            at java.util.ArrayList$Itr.checkForComodification(Unknown Source) ~[?:1.8.0_231]
            at java.util.ArrayList$Itr.next(Unknown Source) ~[?:1.8.0_231]
            at me.geekbuild.arvenyonRPG_v2.Utils.SkillUtils.leaveSkillbarMode(SkillUtils.java:54) ~[?:?]
            at me.geekbuild.arvenyonRPG_v2.Main.onCommand(Main.java:111) ~[?:?]
            at org.bukkit.command.PluginCommand.execute(PluginCommand.java:45) ~[patched_1.15.2.jar:git-Paper-98]
            ... 17 more

     


    I explain this error the following way: it searches for the HeldItem I just removed. At least that's my assumption.

    So here's my question... pretty simple: How could I get rid of this error?

    Thanks in advance! (sorry for my bad english)
     
  2. Hello,

    You are removing an item while iterating over the list, thats why you have this Exception.
    You should use an iterator to properly remove the item:
    Code (Java):
    public static void leaveSkillbarMode(Player Player) {
        final Iterator<HeldItem> it = HeldItemList.iterator();
        while (it.hasNext()) {
            final HeldItem found = it.next(); // get the next object in list
            if (found.UniqueId.equals(Player.getUniqueId())) {
                Player.getInventory().setItem(found.Slot, found.Item);
                it.remove(); // remove the object from the list
            }
        }
    }
     
  3. Thanks alot, this solved it for me.

    Just that I understand what I am doing - I guess when I remove an Item, the indexes of the following ones change, and therefore aren't the same as the one I was storing beforehand? And an iterator sorts this out...?
     
  4. Instead of using an Iterator, I recommend to use Collection#removeIf(Predicate).

    And please make the fields in your class private and generate getters for them. That is horrible.
     
  5. Can you elaborate? Why would it be better?
    Not that I question you, I just want to understand why I should do this.
     
  6. drives_a_ford

    Moderator

    It's called encapsulation. You can start by reading Beginner Programming Mistakes and Why You're Making Them. The section for encapsulation is rather small there, but I do suggest reading the entire thing if you're just starting out. But you can read more about encapsulation elsewhere, the first answer on Google for me was this one, for instance.
     
  7. If your question was, why I recommend Collections#removeIf(Predicate) over an Iterator:
    It is simply more convinient in my opinion. Some people even go so far to call an Iterator "deprecated". Here is how you could implement it:
    Code (Java):
    heldItemList.removeIf(found -> {
        if (found.uniqueId.equals(player.getUniqueId())) {
            player.getInventory().setItem(found.slot, found.item);
         
            return true;
        }
     
        return false;
    });
    (Fields and local fields should by the way be written in lowerCamelCase, so I changed that.)
     
  8. Wait, is using lambda there is about encapsulation?
     
  9. This solution is slower and use more resource with ArrayList then a simple Iterator.
    Your method is "newer" (Java 8), but does not mean better.
     
    • Agree Agree x 3
  10. drives_a_ford

    Moderator

    Pfft. I thought the question was quoting
    So much for skimming...
     
  11. It doesn't matter if you use an Iterator or Collection#removeIf. If you're starting out at Java, the Iterator is probably more useful to learn. However, if you're an experienced developer, you'll probably want to use Collection#removeIf because it's simpler.
     
    • Like Like x 1
    • Agree Agree x 1