[SPIGOT-6202] Recipes Can't Match Tools/Armor With Durability (With Legacy Plugins Present) Created: 26/Oct/20  Updated: 28/May/24  Resolved: 28/May/24

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

Type: Bug Priority: Minor
Reporter: Nathan Wolf Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: None
Environment:

Mac OS

Version: git-Spigot-57bbdd8-dea4138 (MC: 1.16.3) (Implementing API version 1.16.3-R0.1-SNAPSHOT)
Guidelines Read: Yes

 Description   

Updated: Turns out the problem here is the legacy material system. The example provided did not specify an api-version. Adding an api-version fixes the problem, so this only affects legacy plugins.

I understand if that makes this a "won't fix" bug but I thought I'd submit it anyway.

Using the RecipeChoice API to try to make upgradeable armor, I'd like to be able to use damaged armor.

However, choices that include armor or tools with durability do not work.

Example plugin: https://github.com/NathanWolf/Bukkit-Unit-Tests/tree/crafting-multiple-damaged

This plugin registers a recipe where putting a diamond on top of a leather helmet should yield a diamond helmet.

It works for an undamaged helmet, but not for a damaged one.

I am registering a list of ExactChoice ingredients for the helmet with all 55 damage values:

NamespacedKey recipeKey = new NamespacedKey(this, "irondiamondhelmet");
ShapedRecipe shapedRecipe = new ShapedRecipe(recipeKey, new ItemStack(Material.DIAMOND_HELMET));
shapedRecipe.shape("o", "i");
shapedRecipe.setIngredient('o', Material.DIAMOND);
List<ItemStack> variants = new ArrayList<>();
for (short damage = 0; damage < Material.LEATHER_HELMET.getMaxDurability(); damage++) {
    ItemStack variant = new ItemStack(Material.LEATHER_HELMET, 1, damage);
    variants.add(variant);
}
RecipeChoice.ExactChoice choice = new RecipeChoice.ExactChoice(variants);
shapedRecipe.setIngredient('i', choice);
getServer().addRecipe(shapedRecipe);

The issue seems to stem from here in in CraftRecipe.toNMS:

 

} else if (bukkit instanceof RecipeChoice.ExactChoice) {
    stack = new RecipeItemStack(((RecipeChoice.ExactChoice) bukkit).getChoices().stream().map((mat) -> new net.minecraft.server.RecipeItemStack.StackProvider(CraftItemStack.asNMSCopy(mat))));
    stack.exact = true;
} else {

The CraftItemStack.asNMSCopy method is trying to handle legacy item/data combos and as such drops the damage:

 

Item item = CraftMagicNumbers.getItem(original.getType(), original.getDurability());

This call returns air if the stack passed in has a durability value set for a leather helmet item.

 

Perhaps getItem could special-case Materials that have durability?

I am aware this is somewhat of a silly looking example because I could just use a MaterialChoice for the helmet, but the real goal is to combine this with CustomModelData for a custom crafting system, which is why I need the ExactChoice mechanism.

 

 Comments   
Comment by Nathan Wolf [ 27/Oct/20 ]

I think it's likely to be my code. I've made a lot of invalid assumptions here, I've been trying to fumble my way stepping through Spigot code in a debugger for the first time.

The problem does happen if you're using LEGACY_LEATHER_BOOTS for the crafting recipe. I am still trying to figure out how I get to that from my plugin, which is creating the items based on configs that just say "leather_boots". The ingredients are not serialized item stacks, just me using Material.valueOf().

But probably it's something on my side.

Comment by md_5 [ 27/Oct/20 ]

>I can show that deserializing a legacy item (e.g. one with an item type of WOOD_HOE) forces the legacy material support to initialize. I've updated the test plugin linked above to show that by loading a built-in resource.

Yup that part is to be expected — it's required to turn it from legacy to modern. Sounds like there may be an issue either with the API or your code, but I don't think it's apparent on the basis of this ticket.

Comment by Nathan Wolf [ 27/Oct/20 ]

I apologize for all of the misleading information.

I can show that deserializing a legacy item (e.g. one with an item type of WOOD_HOE) forces the legacy material support to initialize. I've updated the test plugin linked above to show that by loading a built-in resource.

However that does not cause the crafting issue, it won't make the plugin's hard-coded Materials or Material.valueOf suddenly act like it was a legacy plugin. It prints the "Initializing Legacy Material Support" message, I do not really know what other effects it has.

So that must be something weird I'm doing that gets me to having legacy_leather_helmet instead of the modern version.

I think we can close this issue and my PR, unless it the deserialization behavior is surprising enough to track down.

Sorry (and thank you) for your time!

Comment by Nathan Wolf [ 27/Oct/20 ]

Hm, ok thanks, I will look more closely at that then.

Comment by md_5 [ 27/Oct/20 ]

>anytime one of those items loads it switches me to legacy materials.

That also doesn't sound right

Comment by Nathan Wolf [ 27/Oct/20 ]

You are correct, of course. I have to admit it didn't really make sense to me but I must've gotten my testing versions mixed up.

So this only affects legacy plugins, I suppose it's not worth fixing then.

It is an issue for my plugin, even though I do specify an API version I have (for better or worse) let server owners save items from in-game to Yaml so anytime one of those items loads it switches me to legacy materials. I think that is what is causing the crafting to fail for me, anyway.

Comment by Nathan Wolf [ 27/Oct/20 ]

It’s very possible you’re correct there. I’ll repeat my test using two plugins, having the main one specify an api version and see if I can reproduce it.

Comment by md_5 [ 27/Oct/20 ]

I think something is wrong with your understanding, merely loading the legacy system shouldn't change the results as all interactions with that system would load it to begin with — there is no 'is loaded' check anywhere in the server.

Comment by Nathan Wolf [ 27/Oct/20 ]

I created a PR with a fix that works for me, if it is acceptable. I don't see how it could cause any negative side-effects.

https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/pull-requests/764/overview

Comment by Nathan Wolf [ 26/Oct/20 ]

One other note on this, for plugins using item serialization, just loading an old serialized item is also enough to trigger the legacy material system.

So perhaps this is worth fixing? I will test out adding a special-case for items with durability and see if that fixes the issue.

Generated at Tue Apr 22 05:44:22 UTC 2025 using Jira 10.3.5#10030005-sha1:190c783f2bd6c69cd5accdb70f97e48812a78d14.