[MC-131290] Enchantments are saved as shorts, but are loaded as and function with integer values

The bug

The game saves natural enchantments as shorts, yet using integer values (eg 32768 (max short + 1)) makes the game load them fine as integers numbers.

Note that the enchantment saving and loading logic exists in multiple methods which all behave differently.

20w07a Mojang names:

ItemStack.enchant(Enchantment, int): Casts the int enchantment level to byte (-128 to 127) and then stores it as short
EnchantmentHelper.setEnchantments(...): Casts the int enchantment level to short (-32768 to 32767) and then stores it as short
EnchantmentHelper.getItemEnchantmentLevel(Enchantment, ItemStack): Reads as int and then clamps it to 0 to 255
EnchantmentHelper.deserializeEnchantments(ListTag): Reads as int
EnchantmentHelper.runIterationOnItem(EnchantmentVisitor, ItemStack): Reads as int
EnchantedBookItem.addEnchantment(ItemStack, EnchantmentInstance): Reads as int, then writes as short
ItemStack.appendEnchantmentNames(List<Component>, ListTag): Reads as int

How to reproduce

/give @s stick{Enchantments:[{id:"sharpness",lvl:32768}]}

Close and reopen the game
:info: Note that the enchantment level was loaded just fine
Put the stick in an anvil and rename it
❌ The level overflows and becomes -32768

Linked issues

relates to 2

MC-118857 Effect amplifier is read/written as byte from NBT despite being stored as an integer Resolved

MC-135506 Experience saves as a short, used as integer Resolved

Comments 11

NeunEinser 2018-06-13T14:37:19Z

Okay, the code that is responsible for adding an enchantment is very weird.
This code is from 1.12 using mcp 940

net.minecraft.item.ItemStack.addEnchantment(Enchantment, int)

/**
 * Adds an enchantment with a desired level on the ItemStack.
 */
public void addEnchantment(Enchantment ench, int level)
{
	if (this.stackTagCompound == null)
	{
		this.setTagCompound(new NBTTagCompound());
	}
	
	if (!this.stackTagCompound.hasKey("ench", 9))
	{
		this.stackTagCompound.setTag("ench", new NBTTagList());
	}
	
	NBTTagList nbttaglist = this.stackTagCompound.getTagList("ench", 10);
	NBTTagCompound nbttagcompound = new NBTTagCompound();
	nbttagcompound.setShort("id", (short)Enchantment.getEnchantmentID(ench));
	nbttagcompound.setShort("lvl", (short)((byte)level));
	nbttaglist.appendTag(nbttagcompound);
}

The method accepts an int as parameter, then when adding it casts it to a byte, and then stores it as a short. Only thing that's missing here is a long.

Edit
The code in 1.13-pre1 looks very similar. This code was decompiled using the CFR decompiler.

awr.a(azq, int)

public void a(azq azq2, int n2) {
	this.o();
	if (!this.g.c("Enchantments", 9)) {
		this.g.a("Enchantments", new hd());
	}
	hd hd2 = this.g.d("Enchantments", 10);
	gx gx2 = new gx();
	gx2.a("id", String.valueOf(azq.b.b(azq2)));
	gx2.a("lvl", (short)((byte)n2));
	hd2.a(gx2);
}

the o() method seems to initialize the stackTagCompound field (g) if it is null. That's what has been there before, just in its own method. Other than that, there where some changes to the tag itself: It was renamed to Enchantments and is now using a String-ID.

marcono1234 2018-07-25T22:41:20Z

@@unknown, it looks like this method is (at least in 1.12.2) only used for cases where the level is restricted to maximum values of the enchantments anyways (enchantment table, loot tables, /enchant ...), so it is not such a big problem.