Fill Recipe still struggles with NBT

[fzzyhmstrs]

Have a recipe that the fill recipe button knows is valid for filling, but the actual action doesn't do anything:

Chestplates have various nbt and damage etc, and the scepter that goes in the middle has a bunch of nbt.

Recipe filling does work for other recipes in the table with "plain" ingredients.

[emilyploszaj]

Isn't the fill recipe button handled by your recipe handler? Either way I imagine it's delegating down to EMI's base implementation and that should potentially be adjusted, but performance concerns are serious there, so I'm unsure how in depth it can be.

[fzzyhmstrs]

Yes, I am using the StandardRecipeHandler for this.

I could, of course, overwrite canCraft, but I think this behavior isn't "expected" by default. I haven't tested, but I'll hazard a guess that recipes like "damaged item A + damaged item B = less damaged item" run into the same issue; unless they either can't be transferred or have custom canCraft behavior.

Edit: those damage repair recipes don't have a transfer button. See below for my continued experimentation on the issue.

StandardRecipeHandler just delegates to EmiPlayerInventory#canCraft, which uses a HashMap for inventory storage; checking for availability via containsKey, so Stack equality is checked only via hashCode, which in EmiStack just delegates to getKey().hashCode()

https://github.com/emilyploszaj/emi/blob/44b478054a05f96dc935672566653596e49d1678/xplat/src/main/java/dev/emi/emi/api/recipe/EmiPlayerInventory.java#L181

So in canCraft, stack comparisons are ignored completely. However, in EmiRecipeFiller, if I'm following getStacks properly, they are not (isEqual checking both stacks comparisons if the key check passes):

https://github.com/emilyploszaj/emi/blob/44b478054a05f96dc935672566653596e49d1678/xplat/src/main/java/dev/emi/emi/registry/EmiRecipeFiller.java#L151

Which would match my experienced behavior of the craft button showing "This can craft", but the actual fill not working.

[fzzyhmstrs]

Maybe you already sussed all that, hence the note about performance implications, but I'm recording it for my benefit too. Or maybe I'm off base. shrug emoji

That would mean that overriding canCraft wouldn't be correct, I'd have to override craft, and provide my own stack-finder implementation.

That's a tad frustrating; someone else can define stack comparison behavior for a item that happens to be in one of my recipes, and break my normal behavior of "this recipe doesn't know what NBT is"

I'm not sure exactly what a clean solution would be. One potential I just pulled out my ear is a defaulted craftPredicate method that-

returns a BiPredicate<ItemStack, EmiStack> for use by getStacks. With the default being derived from the current line of EmiStack.of(s.getStack()).isEqual(stack). In this way a handler can override the predicate passed to getStacks, to whatever behavior they need for actual filling (say, (itemStack, emiStack) -> itemStack.getItem() == emiStack.getKey()).

(It should just use the built-in Comparison, upon reflection)

It wouldn't resolve the fact that canCraft by it's current nature almost never matches 1:1 with craft fill behavior.

This would be the handler providing the stack comparison instead of the stacks.

Edit: I'm unsure comparisons are actually root cause after the investigation below, but it does still seem like a clear avenue of inequality in behavior, with say Enchanted Books or Potions. I really have no idea what the root cause is after my experiements tbh.

[fzzyhmstrs]

Sorry for now triple commenting, but this issue has haunted me and my poor table for literal years now. Ideally, canCraft and craft conform to identical comparisons in practice. If craft uses Comparisons, canCraft should too, etc. (and ideally the transfer handler and/or EmiRecipe define what Comparison to care about, not the filler method)

If the comparison is only performed on stacks that pass containsKey, this should minimize performance loss to the ingredients that would already be candidates for comparison via key equality.

[fzzyhmstrs]

Another bout of info... so I couldn't replicate this with a random crafting recipe. I also couldn't replicate it with one of the other recipes using my table: This transferred fine with all manner of random NBT on the armor that goes in the middle (I threw out the inv before screenshot, hence the greyed out button).

THIS recipe... both succeeds, aaand fails. Basically if all 4 shields are equivalent internally to each other, the fill succeeds. They can have NBT, but each one has to have exactly identical NBT, or none. Differing NBT and some with/some without NBT isn't tolerated.

With this inventory, the recipe succeeds at transferring via button, all 4 shields having exactly Unbreaking 3:

In this inventory, it fails to transfer. Each shield has a different level of unbreaking:

As I would have hoped, 4 plain shields did work too.

The recipe up top succeeds because there is only 1 NBT-having item, so it will be equivalent to the 0 other instances of itself it would have been checked against.

With this in mind I went back to a vanilla recipe, and voila, can replicate a fail state with planks all named something random: This, "Oak Plankssss", and "Oak P" could NOT transfer with the Oak Slab transfer button; a full copied stack of "Oak Not Planks" did transfer.

[TheRealWormbo]

This sounds a lot like something Immersive Engineering has issues with as well, causing Botania's crafting halo items to not properly process IE recipes with fluid-holding items. According to BluSunrize's response in this issue, it's a vanilla recipe matching limitation. If you are using recipe matching logic inherited from vanilla, you might be running into that same issue.