PaperMC / PaperMC/Paper

MerchantRecipe is inconsistent when calling setters

Open
#10,708 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: accepted type: bug version: 1.20.6
Dominant language
Java
Stars
12.7k
Forks
3.5k
Avg merge
3d 13h
Merged PRs (30d)
11

Description

Expected behavior

When you get a MerchantRecipe from a villager and call one of the setters on it, either they should not be reflected without setting back the recipe to the villager, or all of the setters should be reflected. In other words, it should be consistent one way or another.

Observed/Actual behavior

Some of the setters mutate the actual nms MerchantOffer, while others only take effect after a "refresh" of the trades by actually making a trade.

Steps/models to reproduce

Use the code below in a test plugin


@EventHandler
public void onEvent(PlayerChatEvent event) {
    final Location loc = event.getPlayer().getTargetBlockExact(5).getLocation();
    final Villager villager = event.getPlayer().getWorld().spawn(loc.toCenterLocation().add(0, 1.5, 0), Villager.class);
    villager.setProfession(Villager.Profession.CLERIC);
    final MerchantRecipe recipe = new MerchantRecipe(new ItemStack(Material.DIAMOND, 64), 0, 100, false);
    recipe.addIngredient(new ItemStack(Material.STICK, 10));
    Bukkit.getServer().getScheduler().runTaskLater(this, () -> {
        villager.setMemory(MemoryKey.JOB_SITE, loc);
        villager.setRecipes(List.of(recipe));
    }, 50L);
}

@EventHandler
public void onEvent(PlayerInteractEvent event) {
    final ItemStack item = event.getItem();
    if (item != null && item.getType() == Material.STICK) {
        final Villager villager = event.getPlayer().getLocation().getNearbyEntitiesByType(Villager.class, 5).iterator().next();
        final MerchantRecipe recipe = villager.getRecipe(0);
        recipe.setExperienceReward(true);
        recipe.setIngredients(List.of(new ItemStack(Material.STICK, 5)));
    }
}
  1. Look at brewing stand and type in chat
  2. Wait a few seconds, then check that the villager only has 1 trade (10 sticks -> 54 diamonds). If it's not there, just try again.
  3. Get some sticks and make a trade. NOTE how there is no xp rewarded.
  4. hold a stick and hit the ground
  5. Open the trade screen again (notice how its still 10 sticks)
  6. Make another trade, it should only take 5 sticks because the act of interacting with the trade screen refreshes the changes made
  7. XP is rewarded indicating the trade was changed, but the visuals only change after you actually interact. If you close and re-open now, you should see the changed trade.
Plugin and Datapack List

just a test plugin

Paper version

f041f48458e6d81a54895e988867c0b738922d4d

Other

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the supplied Java plugin reproduction, focusing on Villager#getRecipe, Villager#setRecipes, and the MerchantRecipe setters setExperienceReward and setIngredients. Trace when setter changes reach the villager's trade data and are refreshed for the trade screen. Done means all MerchantRecipe setters consistently take effect at the same point, with regression coverage for the reported behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.