-
-
Notifications
You must be signed in to change notification settings - Fork 348
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
avoid weird itemstacks chosen for ore #10470
base: version/main
Are you sure you want to change the base?
Conversation
@@ -689,13 +689,15 @@ private void discoverOres(final ItemStack stack) | |||
{ | |||
if (stack.is(Tags.Items.ORES) || stack.is(ModTags.breakable_ore) || stack.is(ModTags.raw_ore)) | |||
{ | |||
if (stack.getItem() instanceof BlockItem) | |||
final ItemStack smeltingResult = MinecoloniesAPIProxy.getInstance().getFurnaceRecipes().getSmeltingResult(stack); | |||
if (smeltingResult != null && !ItemStackUtils.compareItemStacksIgnoreStackSize(smeltingResult, stack)) |
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.
smeltingResult
should never be null, it returns ItemStack.EMPTY
on failure to find a recipe.
Other than that (which seems like a bug) this code seems equivalent to the prior version.
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.
The code is reversed, so it checks for a smelting recipe first, and marks it as being broken in other cases
Which solves the issue for raw redstone ore and ancient debris, but will probably give issues for regular ore blocks
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.
It's just a sanity check, so we don't add sth that just becomes itself here as a smeltable item. The if is still checked for both items/blocks
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.
The code is reversed, so it checks for a smelting recipe first, and marks it as being broken in other cases
The reversed order makes no difference, because the two checks are not exclusive and operate on different output collections.
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 missed that the else
was removed, which solved the issue for regular ores
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.
It wasn't removed; it was never there. Hence why I'm saying that this code effectively did not change behaviour at all.
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 added an else temporarily there, but removed it afterwards. This code change just avoids blocks with buggy smelting recipes from being added as a smeltable ore.
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 happened to see the version with the temporary else
Closes #
Closes #
Changes proposed in this pull request
Testing
Review please