Custom IRecipe eats items?

[drok0920]

Hello i have tried to use IRecipe to add NBT to an item when it is crafted.  However the output item is never shown and the items i put in as components disappear when i take them out.  Why is this happening?  Am i doing something wrong?

 

package net.drok.poverhaul.recipe;

import net.drok.poverhaul.ModRegistry;
import net.drok.poverhaul.item.ItemRock;
import net.drok.poverhaul.item.ItemStoneCelt;
import net.minecraft.inventory.InventoryCrafting;
import net.minecraft.item.ItemStack;
import net.minecraft.item.crafting.IRecipe;
import net.minecraft.util.NonNullList;
import net.minecraft.world.World;
import net.minecraftforge.oredict.OreDictionary;

public class RecipeStoneCelt extends net.minecraftforge.fml.common.registry.IForgeRegistryEntry.Impl<IRecipe> implements IRecipe {
	
	@Override
	public boolean matches(InventoryCrafting inv, World worldIn) {
		int rocks = 0;
		int sticks = 0;
		int other = 0;
		
		for(int i = 0; i < inv.getSizeInventory(); i++) {
			
			ItemStack stack = inv.getStackInSlot(i);
			int[] ids = OreDictionary.getOreIDs(stack);
			for (int o = 0; o < ids.length; o++) {
				String ore = OreDictionary.getOreName(ids[o]);
				if(stack.getItem() == ModRegistry.items.get("rock")) {
					rocks++;
				}else if(ore.equals("stick")) {
					sticks++;
				} else {
					other++;
				}
			}
		}
		
		if(rocks == 1 && sticks == 1 && other == 0) return true;
		return false;
	}

	@Override
	public ItemStack getCraftingResult(InventoryCrafting inv) {
		ItemStack rockStack = ItemStack.EMPTY;
		
		for(int i = 0; i < inv.getSizeInventory(); i++) {
			if(inv.getStackInSlot(i).getItem() == ModRegistry.items.get("stonecelt")) {
				rockStack = inv.getStackInSlot(i);
				break;
			}
		}
		
		ItemStack celtStack = new ItemStack(ModRegistry.items.get("stonecelt"));
		ItemStoneCelt celt = (ItemStoneCelt) ModRegistry.items.get("stonecelt");
		ItemRock rock = (ItemRock) ModRegistry.items.get("rock");
		
		celt.setQuality(celtStack, rock.getQuality(rockStack));
		
		return celtStack;
	}

	@Override
	public boolean canFit(int width, int height) {
		return (width >= 2 && height >= 2);
	}

	@Override
	public ItemStack getRecipeOutput() {
		return new ItemStack(ModRegistry.items.get("stonecelt"));
	}

	@Override
	public NonNullList<ItemStack> getRemainingItems(InventoryCrafting inv) {
		NonNullList<ItemStack> nonnulllist = NonNullList.<ItemStack>withSize(inv.getSizeInventory(), ItemStack.EMPTY);

        for (int i = 0; i < nonnulllist.size(); ++i)
        {
            ItemStack itemstack = inv.getStackInSlot(i);

            nonnulllist.set(i, net.minecraftforge.common.ForgeHooks.getContainerItem(itemstack));
        }

        return nonnulllist;
	}
}

 

[Draco18s]

Ok, one, your recipes does dick with NBT data. What you've shown here is that the output stack has the same stack size as one of the input stacks.

Two, ModRegistry.items.get() is not a good way to reference your items, it makes your code very stringly typed.

Three, if you're overriding a method do to the same thing as the parent class, don't override (cough, canFit(), cough).

[drok0920]

First i am dicking with the nbt when i call 

celt.setQuality(celtStack, rock.getQuality(rockStack));

When I do this i am setting an nbt tag called Quality.  Second I dont see why it is bad to use ModRegistry.items.get() as i can get the item based on the registry name and i can register all of the items in one line of code.

[Abastro]

On 2017. 8. 7. at 3:01 AM, drok0920 said:

for (int o = 0; o < ids.length; o++) {

So for every ore ids, you check for the rocks. So it won't work if the rock has no ore id or more than 2 ids.

Also I guess empty itemstack is thought as 'others'...

Plus, when getting the result you look for the last rock slot, while take all of them later. Moreover, I think you remove the result item as well in ::getRemainingItems, but I'm not sure about this part.

Besides, you can use @ObjectHolder.

https://mcforge.readthedocs.io/en/latest/concepts/registries/

[drok0920]

10 minutes ago, Abastro said:

So for every ore ids, you check for the rocks. So it won't work if the rock has no ore id or more than 2 ids.

Also I guess empty itemstack is thought as 'others'...

Plus, when getting the result you look for the last rock slot, while take all of them later. Moreover, I think you remove the result item as well in ::getRemainingItems, but I'm not sure about this part.

Besides, you can use @ObjectHolder.

https://mcforge.readthedocs.io/en/latest/concepts/registries/

First I only check if the itemstack's item is the same as a rock so the oredictionary for that should not be a problem.  Second I didnt think that an empty itemstack would be a problem but now that you mention it i see that it is.  And third i knew that @ObjectHolder was a thing but doesn't it still need to be registered individually?  I am using the Map because i can just convert it's values to an Array and register them all at once.

[Draco18s]

1 minute ago, drok0920 said:

And third i knew that @ObjectHolder was a thing but doesn't it still need to be registered individually?

No, it's annotation magic. All you do is annotate the field (correctly) and everything else happens automagically.

[drok0920]

4 minutes ago, Draco18s said:

No, it's annotation magic. All you do is annotate the field (correctly) and everything else happens automagically.

So if i just created a public static final field then set it to my item it will automatically registered so long as I annotate it?

Edited August 7, 2017 by drok0920

[Draco18s]

2 minutes ago, drok0920 said:

So if i just created a public static final field then set it to my item it will automatically registered so long as I annotate it?

No.

 

You create a field, you annotate it, you register the item normally.

Then forge sets the value of the field for you.

[drok0920]

2 minutes ago, Draco18s said:

No.

 

You create a field, you annotate it, you register the item normally.

Then forge sets the value of the field for you.

Ok then i misunderstood.  If you recommended this instead of the Map that i am currently using then it does not fit what i am looking for.  I am looking for a method where i dont have to manually register each Item via

 

 event.getRegistry().register(value);

 

Is there a better way of doing that or is the only proper way to do each one manually.

[Draco18s]

2 hours ago, drok0920 said:

Is there a better way of doing that or is the only proper way to do each one manually.

Oh you can use a map or list for that, that's fine. You're just iterating over the entries and registering them. But that isn't what I was saying not to do.

 

I was saying, don't do this: if(stack.getItem() == ModRegistry.items.get("rock")) { ... }

 

THAT is what you shouldn't do. It makes your code rely on strings that if you change ("dur, 'rock' such a dumb name I'm going to change that to 'stone_gniess' instead"), well now you have to search your code manually for every instance of the string "rock" and change it by hand. You can't refactor it by using Source -> Refactor -> Rename.

[drok0920]

Oh ok that makes sense one last question since all of my other problems seem to have been fixed is how do i check what item an item stack is?  Do i do stack.equals(stack2)

[Draco18s]

You're already doing it:

stack.getItem() == someItem

[drok0920]

well its never actually evaluating to true

[Draco18s]

Use the debugger to find out why.

[drok0920]

36 minutes ago, Draco18s said:

Use the debugger to find out why.

I figured it out, if the item stack doesnt have an ore dictionary id it never gets to the code that checks if it is a rock.

 

[Draco18s]

9 minutes ago, drok0920 said:

I figured it out, if the item stack doesnt have an ore dictionary id it never gets to the code that checks if it is a rock.

 

Mm, you don't say.

 

5 hours ago, Abastro said:

So for every ore ids, you check for the rocks. So it won't work if the rock has no ore id