[1.21] Use RegistryFriendlyByteBuf for extended menus

[Urkaz]

Extended menus use FriendlyByteBuf, so they can't be used with StreamCodecs to serialize things as they require a RegistryFriendlyByteBuf.

Forge and NeoForge already use RegistryFriendlyByteBuf natively when calling player.openMenu, but Fabric's implementation is more custom on the Architectury side and it creates a FriendlyByteBuf.

This PR changes all FriendlyByteBuf into RegistryFriendlyByteBuf in all places I've seen it related to extended menus.

[CLAassistant]

All committers have signed the CLA.

[Urkaz]

Would this cause any breaking changes? Since if other mods are already using this, then they may have to recompile their mod.

I'm not sure if this will be a breaking change, but I don't know exactly how to check it to confirm it or not 😅

What I can say for sure is that RegistryFriendlyByteBuf extends from FriendlyByteBuf so all functions (writeInt, writeBoolean, writeBlockPos, etc) should still be there to use as if it were a FriendlyByteBuf.

If the type is left as a FriendlyByteBuf in ExtendedMenuProvider.java and MenuRegistry.java, then we can cast the FriendlyByteBuf to RegistryFriendlyByteBuf and use it in StreamCodecs like this:

// Overriding the ExtendedMenuProvider
@Override
public void saveExtraData(FriendlyByteBuf buffer) {
    RegistryFriendlyByteBuf b2 = (RegistryFriendlyByteBuf) buffer;
    ItemStack itemInHand = ItemHelper.getItemInHand(ownerPlayer);
    ItemStack.STREAM_CODEC.encode(b2, itemInHand);
}

// During registration
public static final RegistrySupplier<MenuType<MyMenu>> MY_MENU = MENU_TYPES.register("my_menu", () -> MenuRegistry.ofExtended((id, inventory, buffer) -> {
    RegistryFriendlyByteBuf b2 = (RegistryFriendlyByteBuf) buffer;
    ItemStack itemInHand = ItemStack.STREAM_CODEC.decode(b2);
    return new MyMenu(id, inventory, itemInHand);
}));

This example works, but that will still require the proposed changes in fabric/MenuRegistryImpl.java so the same type of buffer is returned by all mod loaders (Forge and NeoForge already use RegistryFriendlyByteBuf in their implementation), or fabric will throw a ClassCastException because of the invalid cast.

Edit: I forgot to mention that this change may be required in 1.20.6 too, as I think the codec and buffer changes were made in that version, but I may be wrong.

[Jab125]

Would this cause any breaking changes? Since if other mods are already using this, then they may have to recompile their mod.

I'm not sure if this will be a breaking change, but I don't know exactly how to check it to confirm it or not 😅

What I can say for sure is that RegistryFriendlyByteBuf extends from FriendlyByteBuf so all functions (writeInt, writeBoolean, writeBlockPos, etc) should still be there to use as if it were a FriendlyByteBuf.

If the type is left as a FriendlyByteBuf in ExtendedMenuProvider.java and MenuRegistry.java, then we can cast the FriendlyByteBuf to RegistryFriendlyByteBuf and use it in StreamCodecs like this:

// Overriding the ExtendedMenuProvider
@Override
public void saveExtraData(FriendlyByteBuf buffer) {
    RegistryFriendlyByteBuf b2 = (RegistryFriendlyByteBuf) buffer;
    ItemStack itemInHand = ItemHelper.getItemInHand(ownerPlayer);
    ItemStack.STREAM_CODEC.encode(b2, itemInHand);
}

// During registration
public static final RegistrySupplier<MenuType<MyMenu>> MY_MENU = MENU_TYPES.register("my_menu", () -> MenuRegistry.ofExtended((id, inventory, buffer) -> {
    RegistryFriendlyByteBuf b2 = (RegistryFriendlyByteBuf) buffer;
    ItemStack itemInHand = ItemStack.STREAM_CODEC.decode(b2);
    return new MyMenu(id, inventory, itemInHand);
}));

This example works, but that will still require the proposed changes in fabric/MenuRegistryImpl.java so the same type of buffer is returned by all mod loaders (Forge and NeoForge already use RegistryFriendlyByteBuf in their implementation), or fabric will throw a ClassCastException because of the invalid cast.

Edit: I forgot to mention that this change may be required in 1.20.6 too, as I think the codec and buffer changes were made in that version, but I may be wrong.

all RegistryFriendlyByteBufs are FriendlyByteBufs, but not all FriendlyByteBufs are RegistryFriendlyByteBufs, so there may be a problem on fabric.

Also, in Java bytecode, both a method's name and signature are used to figure out what method to call:

saveExtraData(Lnet/minecraft/network/FriendlyByteBuf;)V

so this would probably break all mods that currently use this.

[Urkaz]

I did a few tests on my side and I can confirm that only merging the changes from fabric/MenuRegistryImpl.java doesn't break mods and will continue to work without recompiling. Merging the changes in the other two files (MenuRegistry.java and ExtendedMenuProvider.java) breaks them.

But as I mentioned, those changes are still necessary to fix the custom Architectury's implementation of extended menus for Fabric and make it behave like Forge and NeoForge.

Should I edit this PR with something or is it OK the way it is right now? I'm just asking in case something is blocking it from being merged :) Thanks!

[shedaniel]

I don't think we can afford breaking changes right now, for now you can get the RegistryAccess from the ServerPlayer and create the RegistryFriendlyByteBuf in the consumer yourself, we can merge this change for the next major version so keep this PR open for now.

[Urkaz]

Closing as it is no longer needed in recent versions.