Minecraftforge: [1.13] Item IDs still synced as shorts, which is not enough space in a post-flattening modded world

Created on 9 Sep 2018  路  9Comments  路  Source: MinecraftForge/MinecraftForge

In PacketBuffer, the integer ID of the Item in an itemstack is still written as a short. This limits the number of items to effectively ~32k.

In a world where most mods are expected to flatten their items, this is not enough. A light 40-mod 1.12 pack I'm playing on already uses 6000 item ids. Assuming the worst case where every item had 16 meta variants, this is 96k IDs, too many.

Let's randomly guess that a 100-mod 1.12 pack has 9000 item ids in use (it's likely more than this). Assuming the average case where every item had 8 meta variants, this is 72000 ids flattened, still too many.

The only solution would be for modders to use NBT hackery instead of flattening. But this is highly undesirable since it doesn't mesh well with new systems like the Ingredient system and Tags.

We'll have to find a way to communicate larger IDs between forge servers/clients, and fall back to shorts otherwise.

Though, the best way is to pressure mojang to change this to use Varints in 1.13.2

1.13

Most helpful comment

As long as we keep the vanilla item ID range working the same way, I think we can alter the format for higher IDs, right? Vanilla can't handle the modded items anyway.

So use up all the shorts and then signify that you should read a varint next from the buffer for the ID. You can get fancier by using up the negative short values too (except -1) but that seems like overkill.

```java.patch
+public PacketBuffer writeItemId(int id) {

  • if (id > Short.MAX_VALUE) {
  • this.writeShort(-2);
  • this.writeVarInt(id - Short.MAX_VALUE);
  • } else {
  • this.writeShort(id);
  • }
  • return this;
    +}
    +
    +public int readItemId() {
  • int i = this.readShort();
  • if (i == -2)
  • i = this.readVarInt() + Short.MAX_VALUE;
  • return i;
    +}

public PacketBuffer writeItemStack(ItemStack stack) {
if (stack.isEmpty()) {
this.writeShort(-1);
} else {
Item item = stack.getItem();
- this.writeShort(Item.getIdFromItem(item));
+ this.writeItemId(Item.getIdFromItem(item));
this.writeByte(stack.getCount());
NBTTagCompound nbttagcompound = null;

     if (item.isDamageable() || item.getShareTag()) {
         nbttagcompound = stack.getTag();
     }

     this.writeCompoundTag(nbttagcompound);
 }

 return this;

}

public ItemStack readItemStack() {
- int i = this.readShort();
+ int i = this.readItemId();
if (i < 0) {
return ItemStack.EMPTY;
} else {
int j = this.readByte();
ItemStack itemstack = new ItemStack(Item.getItemById(i), j);
itemstack.setTag(this.readCompoundTag());
return itemstack;
}
}
```

All 9 comments

Assuming the worst case where every item had 16 meta variants, this is 96k IDs, too many.

That is not the worst case. The worst case is far worse than that, items can easily have hundreds of meta variants.

We'll have to find a way to communicate larger IDs between forge servers/clients, and fall back to shorts otherwise.

From personal experience: Couldn't you either:

  • (a) submit an invalid ID number (say, a negative number) as a "ID >~32k, VarInt next in packet",
  • (b) use VarInts if the highest ID in the registry is >~32k?

a) won't work in the cases where multiple entries are sent back-to-back though

for (b), how is the reader supposed to know ahead of time if it's going to be a short or varint? on the network, it's just a bytestream

As long as we keep the vanilla item ID range working the same way, I think we can alter the format for higher IDs, right? Vanilla can't handle the modded items anyway.

So use up all the shorts and then signify that you should read a varint next from the buffer for the ID. You can get fancier by using up the negative short values too (except -1) but that seems like overkill.

```java.patch
+public PacketBuffer writeItemId(int id) {

  • if (id > Short.MAX_VALUE) {
  • this.writeShort(-2);
  • this.writeVarInt(id - Short.MAX_VALUE);
  • } else {
  • this.writeShort(id);
  • }
  • return this;
    +}
    +
    +public int readItemId() {
  • int i = this.readShort();
  • if (i == -2)
  • i = this.readVarInt() + Short.MAX_VALUE;
  • return i;
    +}

public PacketBuffer writeItemStack(ItemStack stack) {
if (stack.isEmpty()) {
this.writeShort(-1);
} else {
Item item = stack.getItem();
- this.writeShort(Item.getIdFromItem(item));
+ this.writeItemId(Item.getIdFromItem(item));
this.writeByte(stack.getCount());
NBTTagCompound nbttagcompound = null;

     if (item.isDamageable() || item.getShareTag()) {
         nbttagcompound = stack.getTag();
     }

     this.writeCompoundTag(nbttagcompound);
 }

 return this;

}

public ItemStack readItemStack() {
- int i = this.readShort();
+ int i = this.readItemId();
if (i < 0) {
return ItemStack.EMPTY;
} else {
int j = this.readByte();
ItemStack itemstack = new ItemStack(Item.getItemById(i), j);
itemstack.setTag(this.readCompoundTag());
return itemstack;
}
}
```

That solution looks like pretty much what Lex and I were discussing. However it will need to go hand-in-hand with rejecting vanilla connections if the server item registry is >Short.MAX_VALUE in size.

PlayerHere #MaybeStupid Most vanilla connections are rejected anyway from modded servers as they not have the required mods. Is it possible to connect a vanilla client to a server with mods that has > Short.MAX_VALUE item registry without being rejected with "Missing mods" error? Or these are communicated in different stages of the connection?

Though, the best way is to pressure mojang to change this to use Varints in 1.13.2

Seems they did do this, see https://wiki.vg/Pre-release_protocol.

closing then, since it's unlikely forge will be ready for widespread use before 1.13.2 releases in several days, so this will be resolved in all 1.13.x forges

Is this thing outdated?

If Minecraft Forge is installed and a sufficiently large number of blocks are added, the bits per block value for the global palette will be increased to compensate for the increased ID count. This increase can go up to 16 bits per block (for a total of 4096 block IDs; when combined with the 16 damage values, there are 65536 total states). You can get the number of blocks with the "Number of ids" field found in the RegistryData packet in the Forge Handshake. (https://wiki.vg/Chunk_Format)

Was this page helpful?
0 / 5 - 0 ratings