-
Notifications
You must be signed in to change notification settings - Fork 286
Fix Chemical Reactor NPE and other recipe bugs, version number issues #2316
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
Fix Chemical Reactor NPE and other recipe bugs, version number issues #2316
Conversation
slava110
left a comment
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.
IMO this fix is kinda dirty. Reloading recipes when server is starting. Other machines don't do stuff like that but this one is special. Also how would it work with ARTweaker which only executes scripts on loading?
src/main/java/zmaster587/advancedRocketry/tile/multiblock/machine/TileChemicalReactor.java
Outdated
Show resolved
Hide resolved
src/main/java/zmaster587/advancedRocketry/tile/multiblock/machine/TileChemicalReactor.java
Outdated
Show resolved
Hide resolved
src/main/java/zmaster587/advancedRocketry/tile/multiblock/machine/TileChemicalReactor.java
Outdated
Show resolved
Hide resolved
slava110
left a comment
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.
Oh, wait, I thought this is per commit. Sorry. GitHub is weird.
Recipe fix looks dirty imo. Should be fixed in LibVulpes instead if it doesn't allow special recipes, no?
|
Lol, first time reviewing stuff so don't judge me c: |
i'm starting to think you didn't saw Brycey's diary on discord about this lol |
Depends on what you're talking about. He asked if it's possible to add CraftTweaker recipes after world load but they should be registered on MC loading. |
|
I can find a way to selectively clear and reload these armor recipes, but I haven't found a way to generate the recipes with enchantments before the enchantment IDs are generated by world-load. |
Moving special recipe processing to LV was the only change I didn't apply, but I now have the Chemical Reactor reloading just the special armor recipes, to prevent problems with CraftTweaker and XML recipes.
I now clear and reload JUST the special armor recipes instead of all of the ones for the machine. Still a dirty fix, but better until we figure out a final solution. |
|
Btw, what happens if the mod author decide to change or remove the recipe? will it cause any issue with this? |
If someone else changes or removes an armor airtight seal recipe in another mod or in Crafttweaker, my changes will negate that. I could potentially redo it so that external changes are kept. |
I re-implemented it 5 more times and finally got it to keep removals of special recipes, but making it keep changes would require a LOT of extra code or a real fix at the source of this problem. Recipes removed and re-created in CT and other mods will be kept, so that's a way to "change" the recipes. It's notable that any time the command is run to reload recipes, all AR recipes will be reset and reloaded from generation and XML. Changes, removals, and additions by other mods will be wiped out until next game load. This was the existing behavior before I began making changes to the Chemical Reactor, and I haven't touched it. |
|
Looks like the reloadRecipes AR command can't work in 1.12.2 the way it's designed: |
No description provided.