gnembon / fabric-carpet

Fabric Carpet
MIT License
1.71k stars 277 forks source link

[Scarpet] parse_nbt parses Byte, Int, and Long Arrays as lists of numbers #1289

Open chililisoup opened 2 years ago

chililisoup commented 2 years ago
nbt = nbt('{ByteArray:[B;1b,2b,3b],IntArray:[I;1,2,3],LongArray:[L;1l,2l,3l]}');
nbt = parse_nbt(nbt);
print(nbt); //{LongArray: [1, 2, 3], ByteArray: [1, 2, 3], IntArray: [1, 2, 3]}
nbt = encode_nbt(nbt);
print(nbt); //{ByteArray:[1,2,3],IntArray:[1,2,3],LongArray:[1,2,3]}

I encountered this when writing scripts to modify the display tag of items. After replacing the item with the modified one, I found that any attributes the item had were broken due to the attribute UUID becoming a normal list of numbers.

One possible fix is parsing those arrays as a map with a 'type' tag (would store 'byte_array', 'int_array', or 'long_array') and a 'value' tag (would store the actual list), then use that to get back to the correct array type with encode_nbt. That would look like this (when applied to my example nbt):

{
    'ByteArray' -> {
        'type' -> 'byte_array',
        'value' -> [1,2,3]},
    'IntArray' -> {
        'type' -> 'int_array',
        'value' -> [1,2,3]},
    'LongArray' -> {
        'type' -> 'long_array',
        'value' -> [1,2,3]}
}

Another possible fix would be based on tag names, for example if encode_nbt were to encounter a 'UUID' tag holding an list of integers with a length of 4, it would encode that as an Int Array. The downside of this method is any tags not recognized by encode_nbt would not be encoded correctly (ie., custom tags)

altrisi commented 2 years ago

The Scarpet language doesn't have know the concept of different number types, there's only one, so once it gets into a scarpet type it'll loose type information (internally not all of it for efficiency, but in practice it will).

IIRC if you create the nbt tag value with the type right new to it (so like 1b, 0b, etc) it should be preserve as long as you don't convert it into a scarpet type again. Didn't know about the type-for-whole-array notation, though if it works with minecraft it should work from scarpet (as long as the previous is followed), that may be another issue.

ch-yx commented 2 years ago

i had said that before #1251 @altrisi

i m thinking about adding a new function called nbt_type

nbt_type(tag , (optional) path)

eg: a=nbt('{a:1,b:2b,c:3.0f}'); nbt_type(a) => 'compound' nbt_type(a,'a') =>'int' nbt_type(a,'b') =>'byte' nbt_type(a,'c') =>'float'

what do you think?

altrisi commented 2 years ago

I think that the NBT system in scarpet should be changed to be more integrated with native types, instead of being a single type that can be multiple things.

I'm not 100% sold in the idea of the nbt_type function, mostly because of having a complete function just for NBTs, but IINW the rest of operators are already used, so there's not really much that can be done without breaking backwards compatibility.

Also that doesn't really relate to this issue, given this issue (from what I understood) talks about encoding scarpet values into a specific NBT (number) type, not getting the type of existing ones.

ch-yx commented 2 years ago

Also that doesn't really relate to this issue, given this issue (from what I understood) talks about encoding scarpet values into a specific NBT (number) type, not getting the type of existing ones.

you need to know what type to convert to in order to preserve it. We don't even have a way now to quickly and safely know if a value in a compound is byte or int, unless we implement a snbt parser ourselves ...........

As for creating an object of a certain type, it is rather simple. Just use something like nbt(input_number+'b').