Fabric API

Fabric API

Mods
152M Downloads

Crash due to `TradeOffersTypeAwareBuyForOneEmeraldFactoryMixin` incorrectly assuming null trade items will create empty item stacks

Antikyth opened this issue ยท 0 comments

commented

This method in the mixin assumes that an empty item stack will be created due to null being passed in the constructor.

/**
 * To prevent "item" -> "air" trades, if the result of a type aware trade is air, make sure no offer is created.
 */
@Inject(method = "create", at = @At(value = "NEW", target = "net/minecraft/village/TradeOffer"), locals = LocalCapture.CAPTURE_FAILEXCEPTION, cancellable = true)
private void failOnNullItem(Entity entity, Random random, CallbackInfoReturnable<TradeOffer> cir, ItemStack buyingItem) {
    if (buyingItem.isEmpty()) { // Will return true for an "empty" item stack that had null passed in the ctor
        cir.setReturnValue(null); // Return null to prevent creation of empty trades
    }
}

Unfortunately, at least in 1.20.1, item stack constructors take ItemConvertible and call asItem on the input.

@Nullable
@Override
public TradeOffer create(Entity entity, Random random) {
    if (entity instanceof VillagerDataContainer) {
        ItemStack itemStack = new ItemStack((ItemConvertible)this.map.get(((VillagerDataContainer)entity).getVillagerData().getType()), this.count);
        return new TradeOffer(itemStack, new ItemStack(Items.EMERALD), this.maxUses, this.experience, 0.05F);
    } else {
        return null;
    }
}

Which causes a null pointer exception if a null item is given.

I imagine this was not noticed until now because in vanilla, the only type aware trade is when a fisherman is at its maximum level and a mod has added a custom villager type. My custom villager type also has its own type aware trades at a lower level though, so this crash happened without max level - but I don't think this could have ever worked, at least with how TypeOffersTypeAwareBuyForOneEmeraldFactory is implemented in 1.20.1.

I suppose to fix this the mixin will have to add a condition before the item stack is created in TypeOffersTypeAwareBuyForOneEmeraldFactory#create, since the mixin disables vanilla's crash in the TypeOffersTypeAwareBuyForOneEmeraldFactory constructor for missing trades.