This PR fixes #1429 by changing ItemScrollData.Mutable from using List#remove for removing ItemStacks from the list, to using a manual iteration and comparing stacks using ItemStack#isSameItem (the same comparison method as ItemScrollData#contains).
As ItemStacks do not implement Object#equals properly, no two instances of an ItemStack will ever satisfy Objects#equal (in the default implementation from Object), even if their contents are identical since their identities are different. This is even more pronounced for ItemScrollData.Mutable#remove, which copies the stack before removing it; thus, it will never match an existing stack in the list.
To work around this limitation, we now manually iterate the list and use ItemStack#isSameItem (mirroring the same method used in ItemScrollData#contains) to compare against the target stack, and removing by index. Using the index for removal means the ArrayList won't have to iterate again in its own code to find the object's index (as it does in ArrayList#remove(Object)).
Some comments have been added to make it clear why that loop exists, to help out in the future when debugging this area of code.
Alternatively, List#removeIf could be used instead of the loop to save on code, but it brings with the side-effect that all instances in the list matching the stack is removed, rather than the first instance which is the behavior of List#remove.
This PR fixes #1429 by changing
ItemScrollData.Mutablefrom usingList#removefor removingItemStacksfrom the list, to using a manual iteration and comparing stacks usingItemStack#isSameItem(the same comparison method asItemScrollData#contains).As
ItemStacks do not implementObject#equalsproperly, no two instances of anItemStackwill ever satisfyObjects#equal(in the default implementation fromObject), even if their contents are identical since their identities are different. This is even more pronounced forItemScrollData.Mutable#remove, which copies the stack before removing it; thus, it will never match an existing stack in the list.To work around this limitation, we now manually iterate the list and use
ItemStack#isSameItem(mirroring the same method used inItemScrollData#contains) to compare against the target stack, and removing by index. Using the index for removal means theArrayListwon't have to iterate again in its own code to find the object's index (as it does inArrayList#remove(Object)).Some comments have been added to make it clear why that loop exists, to help out in the future when debugging this area of code.
Alternatively,
List#removeIfcould be used instead of the loop to save on code, but it brings with the side-effect that all instances in the list matching the stack is removed, rather than the first instance which is the behavior ofList#remove.