Skip to content
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

Use "chestWood" Ore Dictionary values rather than hardcoding the item. #3899

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcus8448
Copy link
Contributor

@marcus8448 marcus8448 commented Mar 2, 2020

Closes #3891
BREAKING CHANGE

Also, I think this is incorrect?

if (isSalvage(stack));
{
return stack.copy();
}

@radfast
Copy link
Collaborator

radfast commented Apr 13, 2020

I would prefer to do this without a breaking change. This one is a severe breaking change because almost every Galacticraft add-on adds more rockets with INasaWorkbenchRecipe. And I don't know if all the add-ons are currently being updated - or updated regularly.

Can we not just:

  • check at runtime when pattern-matching the recipe, if the recipe ItemStack is a chest then do an OreDict chest check
  • update the custom JEI plugin.

More generally in my "value system":

Stability of all our add-ons and all the modpacks which incorporate them > the one guy in issue 3891 who would prefer to use some other mod's wooden chest in a recipe in place of a vanilla chest.

@radfast
Copy link
Collaborator

radfast commented Apr 13, 2020

Also, I think this is incorrect?

if (isSalvage(stack));
{
return stack.copy();
}

Please explain what is wrong here, maybe I am being slow to understand today?

The intention is to deconstruct a rocket into Galacticraft compressed metals, even if the original rocket was constructed using another mod's OreDicted equivalent. That's the only reasonable way to do this - otherwise every crafted item would need to carry metadata listing its original materials, and there's no provision for that in vanilla / Forge.

If you're saying the code does not achieve the purpose stated above, please explain? :)

@marcus8448
Copy link
Contributor Author

Ok, I'll see if I can make this change without breaking add-ons.
About that line...
There is a semicolon after the if statement, meaning it will return the stack even if isSalvage returns false.

@radfast
Copy link
Collaborator

radfast commented Apr 13, 2020

There is a semicolon after the if statement, meaning it will return the stack even if isSalvage returns false.

oooh! I totally missed that, thank you for spotting.

@micdoodle8
Copy link
Owner

I committed a non-breaking version of your code. I like your code better but we'll wait to combine it with other breaking changes after a promotion.

The drawback to my version is you can't mix and match chest types in the same recipe. The number of recipes to search through would increase by a factor of (chest types)^3 if I were to fix this without your Ingredient solution.

@MJRLegends
Copy link
Contributor

Btw your fixing of item rotations commit contains breaking code already due to the method rename in ClientUtil so any addon with rockets now will have to update anyways so you might as well push this change out now too

@BlesseNtumble
Copy link
Contributor

BlesseNtumble/GalaxySpace#427
Already))

@MJRLegends
Copy link
Contributor

MJRLegends commented Apr 17, 2020

Ye looks like im updating ExtraPlanets, Planet Progressions for MC 1.12.2 later then too

Woo glad to see you around again micdoodle tho

@micdoodle8
Copy link
Owner

Ye looks like im updating ExtraPlanets, Planet Progressions for MC 1.12.2 later then too

Woo glad to see you around again micdoodle tho

Sorry, didn't know anyone was using that method! Radfast reverted the breaking change and created build 250.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NASA Workbench to accept chestWood OreDict chests
5 participants