[BUILDTOOLS-519] Mapping from nms to craftbukkit is incorrect. This lead to bug that villagers get doubled experience on trading. Created: 10/Apr/20  Updated: 23/Jun/20  Resolved: 23/Jun/20

Status: Resolved
Project: BuildTools
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Minor
Reporter: yosihiro Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: BuildTools, Craftbukkit, bug

Version: This server is running CraftBukkit version git-Bukkit-183139d (MC: 1.15.2) (Implementing API version 1.15.2-R0.1-SNAPSHOT)
Guidelines Read: Yes

 Description   

I was confused where to post this issue.
Sorry if this is inappropriate ticket for BuildTools.

Bug reproduction movie is here

Cause of the problem is incorrect source code mapping from nms to craftbukkit.

In nms before mapped, EntityVillager class (actual class name is "axw") has method:

   public void u(int var1) {
      this.bJ = var1;
   }

This method is mapped to:

    @Override
    public void setExperience(int i) {
        this.bJ = i;
    }

 

And then, the super class EntityVillagerAbstract (actual name is "axs") has this method:

   public void t(int var1) {
   }

This method is also mapped to:

    @Override
    public void setExperience(int i) {}

 

At this point, method EntityVillagerAbstract.setExperience(int i) behaves differently from vanilla minecraft.

In vanilla minecraft code, EntityVillager (axw) class do not have method t(int i) which is setExperience(int i) in EntityVillagerAbstract

 

Because of this incorrect mapping, method SlotMerchantResult.a(EntityHuman entityhuman, ItemStack itemstack) gives villagers experience twice. 

SlotMerchantResult.a(EntityHuman entityhuman, ItemStack itemstack) ↓

    @Override
    public ItemStack a(EntityHuman entityhuman, ItemStack itemstack) {
        this.c(itemstack);
        MerchantRecipe merchantrecipe = this.a.getRecipe();

        if (merchantrecipe != null) {
            ItemStack itemstack1 = this.a.getItem(0);
            ItemStack itemstack2 = this.a.getItem(1);

            if (merchantrecipe.b(itemstack1, itemstack2) || merchantrecipe.b(itemstack2, itemstack1)) {
                this.h.a(merchantrecipe); // This line gives experience firstly.
                entityhuman.a(StatisticList.TRADED_WITH_VILLAGER);
                this.a.setItem(0, itemstack1);
                this.a.setItem(1, itemstack2);
            }

            // This line gives experience secondly.
            // In vanilla minecraft, this line do nothing.
            this.h.setExperience(this.h.getExperience() + merchantrecipe.getXp());
        }

        return itemstack;
    }

 

EntityVillager.a(MerchantRecipe merchantrecipe) ↓

    @Override
    public void a(MerchantRecipe merchantrecipe) { // Called method above
        merchantrecipe.increaseUses();
        this.e = -this.A();
        this.b(merchantrecipe); // Calls method below
        if (this.tradingPlayer instanceof EntityPlayer) {
            CriterionTriggers.s.a((EntityPlayer) this.tradingPlayer, this, merchantrecipe.getSellingItem());
        }

    }

    ...

    @Override
    protected void b(MerchantRecipe merchantrecipe) {
        int i = 3 + this.random.nextInt(4);

        this.bJ += merchantrecipe.getXp(); // This line actually gives experience firstly.
        this.bD = this.getTrader();
        if (this.eW()) {
            this.bB = 40;
            this.bC = true;
            i += 5;
        }

        if (merchantrecipe.isRewardExp()) {
            this.world.addEntity(new EntityExperienceOrb(this.world, this.locX(), this.locY() + 0.5D, this.locZ(), i));
        }

    }

 

To resolve this bug, BuildData/mappings/bukkit-1.15.2-members.csrg need to be fixed.
EntityVillager u (I)V setExperience (line 2882)  or IMerchant t (I)V setExperience (line 3402) must be changed its mapping result.

Thanks.

 

 

 



 Comments   
Comment by md_5 [ 23/Jun/20 ]

Fixed in 1.16

Generated at Sat Dec 13 14:30:04 UTC 2025 using Jira 10.3.13#10030013-sha1:56dd970ae30ebfeda3a697d25be1f6388b68a422.