fabric icon indicating copy to clipboard operation
fabric copied to clipboard

Add an item setting for better recipe remainders

Open BoogieMonster1O1 opened this issue 3 years ago • 12 comments

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

gravel sand

Fixes #50

BoogieMonster1O1 avatar Nov 24 '20 08:11 BoogieMonster1O1

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

i509VCB avatar Nov 24 '20 08:11 i509VCB

I guess I'll be the one to ask, where did the texture~~s~~ 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

BoogieMonster1O1 avatar Nov 24 '20 08:11 BoogieMonster1O1

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

i509VCB avatar Nov 24 '20 10:11 i509VCB

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

yes but only the recipe remainder part

BoogieMonster1O1 avatar Nov 24 '20 10:11 BoogieMonster1O1

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 avatar Mar 18 '21 20:03 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.

Technici4n avatar Apr 10 '21 13:04 Technici4n

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

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

Sollace avatar Apr 10 '21 14:04 Sollace

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 avatar Apr 10 '21 14:04 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?

Technici4n avatar Apr 10 '21 14:04 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?

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.

Sollace avatar Apr 10 '21 16:04 Sollace

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?

ZestyBlaze avatar Nov 28 '21 04:11 ZestyBlaze

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

kwpugh avatar Apr 22 '22 13:04 kwpugh

Superseded by #2464.

Technici4n avatar Aug 30 '22 22:08 Technici4n