-
Notifications
You must be signed in to change notification settings - Fork 655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove hard-coded disassembly item from crafting blocks, use recipe IDs instead #7396
base: main
Are you sure you want to change the base?
Conversation
d7bdcbb
to
13c3fcc
Compare
inventory.setItem(inventory.selected, ItemStack.EMPTY); | ||
|
||
for (var ingredient : recipe.get().getIngredients()) { | ||
var ingredientStack = new ItemStack(ingredient.getItems()[0].getItem(), inventory.getSelected().getCount()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have to test for empty ingredient.
Also -> If the ingredient is a tag-ingredient, this may have VERY unintended consequences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might it be a good idea then to outright prevent disassembly if a tag is used for the ingredient? Or, in general, suppressing disassembly if the ingredient can be of more than one item?
} | ||
|
||
private void disassemble(ItemStack stack, Player player) { | ||
protected ResourceLocation getRecipeId() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename -> getDisassembleRecipeId()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I am not sure this reverse lookup is a good idea. It might instead just be a property passed in at item creation time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed this was okay to do from the fact that this is already done for portable cells.
Partly addresses #6934.