Recipe enhancements #311
Merged
Recipe enhancements #311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BuckarooBanzay
approved these changes
Jun 24, 2023
|
Does not have to be changed here but any alloying recipe, that currently results in dust, should be updated to give ingots instead. It just makes sense to return only ingots from smelting and never dust piles. |
S-S-X
reviewed
Jun 27, 2023
| local result = data.output | ||
| if (type(result)=="table") then | ||
| result = result[1] | ||
| if data.hidden then return end |
There was a problem hiding this comment.
Would be good if this return in the middle of function can be refactored to not be here.
Pretty sure this function should be split into two for readability, everything below this line would probably be better split into another local function.
There was a problem hiding this comment.
I don't know, seems unnecessary to split it, even for readability. I think I'll just leave it for now, it can be refactored later when group support is added.
S-S-X
approved these changes
Jun 27, 2023
|
I changed the dust alloy recipes to output ingots, but I just thought, won't it break automated setups? |
Click for detailed source code test coverage reportTest coverage report for Technic CNC 87.36% in 11/14 files:Test coverage report for technic chests 45.49% in 6/6 files:Test coverage report for technic 66.35% in 96/96 files:Raw test runner output for geeks:CNC: Chests: Technic: |
It is possible but I think it is unlikely, what I've seen so far ppl usually seem to be feeding ingots to furnaces and that's probably because ingots are also better for storage (as a blocks) + because of being consistent with primary guide recipes. While it can break setups I don't think it breaks that many setups and I'd say it is so inconsistent anyway that when it breaks some automated setup then that setup should be fixed instead of trying to protect it here. While being backward compatible is generally nice thing, still going too far with protecting existing stuff isn't good for development. Going to be beta release anyway 😁 |
|
Ah, true. I didn't think of that.
Agreed 👍 |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A few minor, but much needed recipe enhancements:
unified_inventory. Purely cosmetic.Example of chernobylite dust centrifuging recipe: