Issue type:

• 🐛 Bug

Short description:

Installing both GTCEu-Modern v1.6.2 and EvilCraft 1.2.49 results in Minecraft:Potion items getting an additional NBT on them. This causes issues with any other mod that adds crafting recipes utilizing a Minecraft:Potion because they expect the one NBT specifying the Potion type.

Steps to reproduce the problem:

1. Install GTCEu-Modern 1.6.2, EvilCraft 1.2.49, JEI v15.20.0.105, and for example Draconic Evolution v3.1.2.604
2. Start a world in Creative
3. Use JEI to bookmark the ingredients for the Damage Module from Draconic Evolution
4. Give yourself the ingredients
5. Hit F3+H and confirm that there are 2 NBT tags on the strength potion
6. Place a Crafting Table down and try to craft the Damage Module

Expected behaviour:

No added NBT to a Minecraft:Potion item so that crafting recipes using the potion still work

Versions:

• This mod: 1.2.49
• GTCEu-Modern: 1.6.2
• Minecraft: 1.20.1
• Mod loader version: Forge 47.3.11

Additional Information:

You don't need to install JEI or Draconic Evolution to reproduce this. Having just GTCEu-Modern 1.6.2 and EvilCraft 1.2.49 is enough to see that potions have an additional NBT stored on them.

Thanks for reporting!

AFAIK, EvilCraft does not add any tags to potions.
Can you report this to the GTCEu-Modern issue tracker and link back here?
Happy to make changes here if the GTCEu-Modern mod author considers it relevant.

Sure thing - I reported it here first since it doesn't happen with just GTCEu-Modern installed and appears to be a mod interaction, but I'll make an issue there too. Issue here

GTCEu-Modern recently added functionality where Potions are now a Fluid, and my thought was the additional NBT could be getting added so the Dark Tank would work properly with it.

What causes the poison bottle to have the NBT Fluid: {FluidName: "evilcraft:poison", Amount: 1000}? With just EvilCraft installed and no other mods. JEI shows a poison bottle with no NBT, cheating one into your inventory changes it to have that additional NBT.

This is what the added NBT looks like on the potions when GTCEu-Modern is also installed.

I suspect something in EvilCraft may be calling the setFluid method on items that have a FluidHandlerItemStack capability, which is writing the Fluid tag to the item. I've made a PR to GTM that overrides the setFluid and getFluid methods here. This resolved the issue in my local testing instance.

I suspect something in EvilCraft may be calling the setFluid method on items that have a FluidHandlerItemStack capability

AFAIK, we don't call this method anywhere: https://github.com/search?q=repo%3ACyclopsMC%2FEvilCraft%20setFluid&type=code
But if you see something I don't, I'd be happy to hear about it.

Alright, I believe I've found where this is stemming from. I'm still not entirely sure I understand everything, but hopefully the below makes sense.

GTM uses the AttachCapabilitiesEvent<ItemStack> event to attach a Fluid Handler Item capability to potions.

if (itemStack.getItem() instanceof PotionItem) {
    LazyOptional<IFluidHandlerItem> handler = LazyOptional.of(() -> {
        var fluidHandler = new FluidHandlerItemStack.SwapEmpty(itemStack, new ItemStack(Items.GLASS_BOTTLE),
                PotionFluidHelper.BOTTLE_AMOUNT);
        fluidHandler.fill(PotionFluidHelper.getFluidFromPotionItem(itemStack, PotionFluidHelper.BOTTLE_AMOUNT),
                IFluidHandler.FluidAction.EXECUTE);
        return fluidHandler;
    });
    event.addCapability(GTCEu.id("potion_item_handler"), new ICapabilityProvider() {

        @Override
        public @NotNull <T> LazyOptional<T> getCapability(@NotNull Capability<T> cap,
                                                          @Nullable Direction side) {
            return ForgeCapabilities.FLUID_HANDLER_ITEM.orEmpty(cap, handler);
        }
    });
}

With just GTM installed, this is fine and does not set the NBT despite the fill method calling the setFluid method in FluidHandlerItemStack because of an attached capability to glass bottles that overrides the setFluid method. You'll notice in particular, after the NBT is set from super.setFluid(fluid), that it overwrites container and thus deletes the NBT data. I believe the general usage scenario here is that you take glass bottles from your inventory and click on a fluid inventory slot in a machine to fill the bottle with the potion. As well as being able to take potion bottles from your inventory and click on a fluid inventory slot in a machine to fill the machine with a potion and give you the glass bottle back.

else if (itemStack.is(Items.GLASS_BOTTLE)) {
    LazyOptional<IFluidHandlerItem> handler = LazyOptional.of(() -> new FilteredFluidHandlerItemStack(itemStack,
            250, s -> s.getFluid().is(CustomTags.POTION_FLUIDS)) {

        @Override
        protected void setFluid(FluidStack fluid) {
            super.setFluid(fluid);
            if (!fluid.isEmpty()) {
                container = PotionUtils.setPotion(new ItemStack(Items.POTION),
                        PotionUtils.getPotion(fluid.getTag()));
            }
        }
    });
    event.addCapability(GTCEu.id("bottle_item_handler"), new ICapabilityProvider() {

        @Override
        public @NotNull <T> LazyOptional<T> getCapability(@NotNull Capability<T> cap,
                                                          @Nullable Direction side) {
            return ForgeCapabilities.FLUID_HANDLER_ITEM.orEmpty(cap, handler);
        }
    });

}

From what I can gather, EvilCraft uses the RenderOverlayEventHook and iterates over the inventory getting the fluid handlers (if present) for the items. As seen above, the attached fluid handler for potions is a LazyOptional and includes the fill method. In this scenario, it will not call the setFluid override provided by the glass bottle's fluid handler, and thus the NBT will remain on the potion.

My PR to GTM resolves this by adding overrides to the setFluid and getFluid methods for the fluid handler attached to potions.

Aha, good catch!

So it looks like this issue would have occurred with other mods as well.

But I'm glad to hear the issue is resolved now :-)