Add an item setting for better recipe remainders

[BoogieMonster1O1]

Adds a conditional recipe remainder (that returns an ItemStack) to FabricItemSettings
also fixes an issue with duplicate keys in the fabric item api v1 testmod fabric.mod.json

Fixes #50

[i509VCB]

I guess I'll be the one to ask, where did the textures for the hammers come from?

[BoogieMonster1O1]

I guess I'll be the one to ask, where did the textures for the hammers come from?

from gimp
i made it for a mod im making
https://github.com/BoogieMonster1O1/ex/blob/main/ex-nihilo-superesse/src/main/resources/assets/ex-nihilo-superesse/textures/item/netherite_hammer.png

[i509VCB]

This kinda sounds like what would supersede #487 I believe?

[BoogieMonster1O1]

This kinda sounds like what would supersede #487 I believe?

yes but only the recipe remainder part

[OroArmor]

Looks good. I would change the name for the test hammer image, but that's a minor difference. The only other thing I would look at is what happens when the item breaks, and if someone wants to replace it with a different item(stack) as that allows for more customization for recipes. As well, a way to make it so that there is a simple method for returning the same item might be nice, as it doesn't require implementing a RecipeRemainderProvider for newer modders and for convenience.

[LambdAurora]

Seems good.

Little nitpick: @param javadocs shouldn't start with capitalized letter and end with a comma like @return javadoc.

[liach]

No critical problems as I can see.

[Technici4n]

Question: do we need to have custom behavior depending on whether the RecipeRemainderProvider is invoked for:

1. A furnace fuel. (A lot of mods will want to use this too);
2. A crafting table or 2x2 crafting inventory. (Some mods will want to use this too);
3. A brewing stand input. (Some mods might want to use this too?)?

I am pretty sure we do, and I believe the arguments could make that more explicit. Maybe we could have an enum for context? Or a RemainderContext interface if we want to be able to change it in the future?

public interface RemainderContext {
    boolean isCrafting();
    boolean isBrewing();
    boolean isFuel();
    // throws if the context is not crafting
    Recipe<?> craftingRecipe();
}

Also, do we really need to have World and BlockPos? It is not because MC provides them that we should have them.

It's also not clear if the original ItemStack may be mutated or not, you should definitely add that to the javadoc.

(I thought about making this an event originally, but this can be added independently later, here we are just extending MC's recipe remainder system to make it more flexible and I think an event would be out of scope).

[Technici4n]

Do we really need the non-ItemStack parameters? Imo the cases where you need access to the inventory or the player are better handled using a custom recipe type.

[Sollace]

    boolean isCrafting();
    boolean isBrewing();
    boolean isFuel();

Why do we need these methods again that can't already be determined using ctx.getRecipeType() == RecipeType.CRAFTING?

[Technici4n]

Last time I checked brewing and fuels have no recipe type...

But tbh I would really prefer an ItemStack extension to getRecipeRemainder: getRecipeRemainder(ItemStack stack), ideally with the added context as well if the behavior needs to be different in these 3 (and possibly more in the future) cases.

[Technici4n]

I can tolerate anything that needs to vary on a) the item stack and b) possibly the context, but if it needs to vary based on the actual recipe it should probably be using a custom recipe type... In general, do we even need this for recipes, or are custom recipe types enough?

[Sollace]

I can tolerate anything that needs to vary on a) the item stack and b) possibly the context, but if it needs to vary based on the actual recipe it should probably be using a custom recipe type... In general, do we even need this for recipes, or are custom recipe types enough?

I think custom recipe types are enough for most use cases.

If you're using this method, chances are it's because you want the behaviour applied to all recipes of a specific kind (crafting/smoking/etc) involving your item. Otherwise you would just make a custom recipe type to use where needed.

[NetUserGet]

Good stuffs :>

[ZestyBlaze]

Is it bad how i actually kind of agree with Technician that having a function in the Item class that extends ItemStack and returns the stack would probably be a lot easier and tidier then doing all that code in creating an item since that's a setting not a function we can override?

[kwpugh]

Question: Is this going to get incorporated into 1.18.x ?

[Technici4n]

Superseded by #2464.