search

KyoriPowered / adventure

A user-interface library, formerly known as text, for Minecraft: Java Edition
https://docs.advntr.dev/
MIT License
678 stars 103 forks source link

`ListBinaryTag` is weakly immutable #1070

Closed mworzala closed 2 months ago

mworzala commented 2 months ago

List stores an immutable view of the input lists, it should do a defensive copy (eg List.copyOf).

var entries = new ArrayList<BinaryTag>();
entries.add(StringBinaryTag.stringBinaryTag("abc"));
entries.add(StringBinaryTag.stringBinaryTag("def"));
var tag = ListBinaryTag.listBinaryTag(BinaryTagTypes.STRING, entries);
entries.clear();
assert tag.size() == 2; // fails
RealBauHD commented 2 months ago

You are right, normally new lists and maps are created for the wrapped immutables, because JEP 269 was introduced in Java 9. I'll take a look at the places where it was forgotten.

RealBauHD commented 2 months ago

It seems that only ListBinaryTag#listBinaryTag uses no copy, have you tested it with the CompoundBinaryTag?

mworzala commented 2 months ago

Yep you're right sorry, compound wraps it correctly I misread.