search

minetest-mods / technic

Technic mod for Minetest
Other
145 stars 155 forks source link

Avoiding alloy furnaces having identical items in both slots #523

Closed int-ua closed 4 years ago

int-ua commented 4 years ago

New version made with SwissalpS, supersedes #521 Fixes #520

int-ua commented 4 years ago

I'm going to publish new commits here but do not merge this PR, I'll squish commits before merging and will create a new one before final merge.

int-ua commented 4 years ago

Is there anything else I can improve before merging?

SmallJoker commented 4 years ago

@int-ua If you're happy with the functionality, I can give it a try again to check whether it works as expected. Code style review will happen in the same step. Will wait the PR is no longer [WIP]. EDIT: Regarding the commit from me - I took the opportunity to try to make it shorter and a bit more precise. Feel free to change the documentation if you'd prefer another text.

int-ua commented 4 years ago

Yes, please do it. I'm keeping [WIP] to avoid merging before squashing commits manually to save SwissalpS' and your contribution in Co-authored-by, ok?

SmallJoker commented 4 years ago

Is it possible to add a partial ItemStack to the alloy furnace's input list? The PR works so far, but the following scenario should be improved if possible:

The 99 tin ingots are rejected, even though they could fill up the slot. EDIT: Feel free to squash the PR. The changes are easy to review in a single commit.

int-ua commented 4 years ago 
* Alloy furnace: [20x tin ingot] [Empty]
* Tube: 99x tin ingot

The 99 tin ingots are rejected, even though they could fill up the slot.

What is the state of "Allow splitting" setting? AFAIU you had it on off and it's working as intended, not splitting incoming stack, no?

EDIT: they should be split with current code with the setting being on.

SmallJoker commented 4 years ago

@int-ua Sorry, I totally forgot about that setting. It indeed works as expected now. There's only a single comment left over, everything else is ready to merge.

SmallJoker commented 4 years ago

1a45ad19