ZenonSeth / logistica

A Minetest item transport and storage mod
Other
1 stars 4 forks source link

Crafting supplier free output with mass storage in pull mode #17

Closed Ocraw closed 1 month ago

Ocraw commented 1 month ago

Crafting supplier dupes (or the mass storage itself, not sure who to blame), if a mass storage set to store the recipe output is in pull mode, the recipe output is taken from the crafting supplier no matter if there are available items in the network. It just pulls the ghost recipe output, leaving it empty, and creating an item for free.

Tested on mineclonia.

ZenonSeth commented 1 month ago

Hm, good catch, I will definitely try to investigate this further and fix, hopefully by end of the weekend.

ZenonSeth commented 1 month ago

It was a problem with the mass storage logic, which was written before crafting suppliers were a thing. Frankly, I need to rewrite much of the code around the network traffic, but for now, this issue is fixed in Release 1.2.2 - and available on ContentDB. Thanks for reporting.

Ocraw commented 1 month ago

Confirm that part is fixed, but another issue raised: put furnace and stack of cobble as pulled items in mass storage; left click with a stack of cobble drawing the furnace recipe in a crafting supplier; you should end up splitting the stack evenly, in mcl case resulting in 8 stacks of 8. Watch the crafting supplier pull all the cobble, producing 10 furnaces, using 8 cobble and putting the 'excess' in its inventory. That excess is 56 cobble. It's crafting 9 furnaces for free.

![Uploading IMG_20240717_001947_299.jpg…]()

Don't think there is ever a case where a recipe uses more than one item on the same stack, correct me if I'm wrong. Could be fixed by forcing the ghost items to be exactly one no matter how much you try to stack in the recipe grid. Would also fix the autocrafter, which while not duping, it just stops crafting after the first iteration, putting excess in its output inventory, if used as described above, with left click drawing.

(Not sure why IMG isn't loaded on my part, hope you can see it)

ZenonSeth commented 1 month ago

The image isn't loading for me either, I think you didn't wait long enough for it to finish uploading, the text still says "Uploading..." -- but I think I got what you meant. It's not a critical issue perhaps, hopefully people aren't putting multiple items per stack in the recipes, but I will look into fixing this.

I also have not seen any recipes with multiple items in a stack, but they are technically possible, so I don't know if I want to limit the ghost items to only 1 per stack, but then it may be a better and easier compromise than a more complicated check or fix.

Ocraw commented 1 month ago

Take your time. You did a great job on logistica, and still doing great on fixing issues. Thanks for your work!

ZenonSeth commented 1 month ago

Take your time. You did a great job on logistica, and still doing great on fixing issues.

thanks, appreciate the feedback, I haven't gotten a lot of feedback aside from a few issues raised.

I'll try to investigate the latest issue you mentioned by the end of the weekend, hopefully sooner.

Ocraw commented 1 month ago

Might be all fixed with your planned rewrite, but better safe than sorry I guess. Still related I think so I keep adding here, if you want me to open a different issue let me know. When you take out a stored amount of "craft recipe output", materials get consumed as if you crafted that amount. Ex. setup crafting supplier to produce a furnace, store a couple of furnaces and cobble, take furnaces out from AP, 16 cobble gets consumed. Also the AP is displaying the number of items you have stored + crafting output(always 1), even if you don't actually have materials to craft that extra item, it's just reading the output as if it was a real item in the network.

ZenonSeth commented 1 month ago

Might be all fixed with your planned rewrite, but better safe than sorry I guess.

I don't have concrete plans to rewrite the network access, it's more that I see issues with how its written that could be improved, but due to time constraints, I'm not going to do so any time soon.

Still related I think so I keep adding here, if you want me to open a different issue let me know.

All of these issues aren't actually related to the original issue you raised here, but it's up to you. Since I'm the only person working on maintaining this, I'm not being pedantic about how issues are raised. The previous bug (where recipe in crafting supplier is entered with stacks of items) also isn't related to the original issue, so it could also be its own bug.

When you take out a stored amount of "craft recipe output", materials get consumed as if you crafted that amount. Ex. setup crafting supplier to produce a furnace, store a couple of furnaces and cobble, take furnaces out from AP, 16 cobble gets consumed.

I was surprised to hear this as it should not happen, so I tested it out, could not reproduce.

image My test setup: a small network with 1 Network Controller, 1AP, 1 CraftSup, 1 Mass Storage, set up as you said - when I took out a furnace from the AP, it took one of the stored furnaces in the mass storage, and used no cobble whatsoever. Could you try to reproduce this with a minimal setup? Also, I assume you're on mineclonia? It shouldn't matter at all for this logic, but I can check there too.

Also the AP is displaying the number of items you have stored + crafting output(always 1), even if you don't actually have materials to craft that extra item, it's just reading the output as if it was a real item in the network.

This is intentional, as it lets you take items from the AP by going through the crafting supplier, thus crafting them for you. If you set up only a network with 1AP, 1CraftSup (say to craft Furnances), but put no cobble in the network, when you try to take out a furnace from the AP, it shows an error below the search field, about it: image

I did have plans to separate these, potentially in tabs (I've raised a feature request to myself about it) but again, due to life and time constraints, I haven't worked on it.

Ocraw commented 1 month ago

Yeah, happens only in mcl2/a, tried in minetest_game and it works fine. Tried with a couple more setups, doesn't seem to make a difference, also on server and singleplayer happens the same. To be more specific I take the items with either shift-left-click or simply left-click, which causes the bug, otherwise, with right-click to drop one at a time, does not happen.

I understand the reasoning behind showing what you can craft, but I would expect it to show first only how many are stored (and only letting me take that amount with a single click), then go in 'craft mode' and start using up materials as I keep requesting that item. I think some mod in mc worked like this and that's why I expected it to behave similarly. Would also eliminate the bug in question entirely I think, letting the network know when to use what. I hope I don't sound like I'm telling you how to write your mod, far from that, I have a small English vocabulary, but I do hope the suggestions are somehow useful.

ZenonSeth commented 1 month ago

I hope I don't sound like I'm telling you how to write your mod, far from that, I have a small English vocabulary, but I do hope the suggestions are somehow useful.

Not at all, the mod is for people to use, so I'm happy to increase the usability of the mod if possible, and as I've said, I've heard very little real feedback, so it's good to get some.

I think because this thread is getting hard to keep track I'm going to make separate issues to keep better track - let me know if this covers your concerns:

  1. [Non-Critical] Crafting Supplier Issue: It takes more materials than single item recipe requires, when recipe specified has more than 1 item per slot I've tested this and as far as I can tell, no free materials are given, only issue is that the crafting supplier pulls more materials and puts them in its extras output slot.
  2. [Critical] Access Point or Network access Issue: taking a full stack (or shift-click for all items) from an AP will cause any crafting supplier to use materials as though the full stack had to be crafted. This to me is critical because it means materials get destroyed unnecessarily
  3. [Enhancement] Access Point Issue: There's no differentiation between stored and craft-sup-provided in the AP. So if there's 10 stored and 1 provided from a CraftSup, then it will make it seem like you can take 11 out of the system. I don't think this breaks anything, but it does make it difficult to take all stored items without forcing anything to craft, so I understand why you brought it up. My personal preference to a solution would be to try and separate how stored and crafting-supplied items are displayed in the AP, but that isn't necessarily easy to achieve, both from a technical point of view, and from a user-friendly UI point of view. Adding different tabs may be the best solution - or actually implementing auto-crafting in the Access Point, which I've thought about, but that is also not a small task (autocrafting the way Factorio lets you click "craft item X" and it will auto-craft all items needed for that item X, recursively). That's been on my personal wishlist of a feature for a long time, but Minetest recipes make it a bit more difficult, and the GUI work for it to be intuitive and useful is challenging too. Will have to think about this.

I think this covers the issues you've raised, correct? I think different tickets are best. Will be looking into the AP issue first, trying it out right now, as it is more critical in my view.

Ocraw commented 1 month ago

1-- Also tied to mcl, retested in mt_game and it's fine there. The issue, in game's prospective, happens if you evenly stack in recipe grid, where a dupe occurs. But yeah, reading and processing how many materials per stack is the code issue I guess. So confirming that. 3-- What about a 'C' overlay on the crafted items image? (Or anything that hints these are to be crafted). Not sure how practical. I like how Factorio handled crafting, no hassle and linear progression, no middle steps might also mean less autocrafters, one might prefer to just craft with AP, which is good on servers. I kinda expect, for some reason, it will be op but I can't say I wouldn't want to see how it would work. (well I hope you understood I was referrig to 3. here, github just presumes I don't know numbers)

2-- is fixed in https://github.com/ZenonSeth/logistica/issues/19

ZenonSeth commented 1 month ago

3-- What about a 'C' overlay on the crafted items image? (Or anything that hints these are to be crafted). Not sure how practical. I like how Factorio handled crafting, no hassle and linear progression, no middle steps might also mean less autocrafters, one might prefer to just craft with AP, which is good on servers. I kinda expect, for some reason, it will be op but I can't say I wouldn't want to see how it would work. (well I hope you understood I was referrig to 3. here, github just presumes I don't know numbers)

Yes, separating the stored items vs craft-able items in the AP grid is one option. Adding an icon will help, of course, but it would be confusing to anyone not reading this thread with the reasons behind it (aka everyone) - so I'd have to add a legend. But the AP interface is already extremely dense with information, which is why I was considering adding tabs. There are other reasons why tabs would be good - maybe move the liquid access to its own tab, allowing mass storage slot allocation/de-alloc, and of course, the advanced crafting.

The AP advanced crafting I was talking about wouldn't totally surpass CraftSups, since AP would not provide its output to machines. It may be slightly OP, which is why I also may hide it behind an upgrade to the AP. Or at least, that's been my plan. But all of this depends on adding tabs first to be honest, which I may not do for a while, considering I have other things to address.

For now, if you are satisfied the original issue you reported here is fixed, could you close this please? If you want to make a ticket for the CraftSups, (item 3), please make one, with any suggestions. We can further discuss it there.