Uploaded image for project: 'BuildTools'
  1. BuildTools
  2. BUILDTOOLS-519

Mapping from nms to craftbukkit is incorrect. This lead to bug that villagers get doubled experience on trading.

XMLWordPrintable

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

      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.

       

       

       

            Unassigned Unassigned
            yoshihiro5225 yosihiro
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: