Galacticraft icon indicating copy to clipboard operation
Galacticraft copied to clipboard

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

Open marcus8448 opened this issue 5 years ago • 9 comments

Closes #3891 BREAKING CHANGE

Also, I think this is incorrect? https://github.com/micdoodle8/Galacticraft/blob/f85e1dd5ff52ca71e3491c609fb524f270bd77fe/src/main/java/micdoodle8/mods/galacticraft/core/tile/TileEntityDeconstructor.java#L383-L386

marcus8448 avatar Mar 02 '20 06:03 marcus8448

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 avatar Apr 13 '20 15:04 radfast

Also, I think this is incorrect?

https://github.com/micdoodle8/Galacticraft/blob/f85e1dd5ff52ca71e3491c609fb524f270bd77fe/src/main/java/micdoodle8/mods/galacticraft/core/tile/TileEntityDeconstructor.java#L383-L386

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? :)

radfast avatar Apr 13 '20 15:04 radfast

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. https://github.com/micdoodle8/Galacticraft/blob/f85e1dd5ff52ca71e3491c609fb524f270bd77fe/src/main/java/micdoodle8/mods/galacticraft/core/tile/TileEntityDeconstructor.java#L383

marcus8448 avatar Apr 13 '20 17:04 marcus8448

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.

radfast avatar Apr 13 '20 18:04 radfast

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.

micdoodle8 avatar Apr 17 '20 07:04 micdoodle8

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

MJRLegends avatar Apr 17 '20 12:04 MJRLegends

https://github.com/BlesseNtumble/GalaxySpace/issues/427 Already))

BlesseNtumble avatar Apr 17 '20 12:04 BlesseNtumble

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

MJRLegends avatar Apr 17 '20 12:04 MJRLegends

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.

micdoodle8 avatar Apr 18 '20 05:04 micdoodle8