When taking items from network, more items may be taken than needed, and then lost

ZenonSeth / logistica

A Minetest item transport and storage mod

Other

1 stars 4 forks source link

When taking items from network, more items may be taken than needed, and then lost #19

Closed ZenonSeth closed 1 month ago

ZenonSeth commented 1 month ago

When taking a stack from the network (for example from an Access Point), the full stack is attempted to be taken from all types of storages when the first storage hit cannot fully fulfil the request. This is dependent upon the internally determined order of storage access.

Example setup on minimal network setup:

Store 10 Stone in a Passive Supply Chest
Store 30 Stone in a Mass Storage node
AP will now show 40 Stone in network (correct)
Attempting to take 20 (half of all present) at once will result in only 10 Stone left over, instead of 20 leftover.

This is caused by internal logic not accounting for partially fulfilled requests correctly.

Created from discussion in https://github.com/ZenonSeth/logistica/issues/17

ZenonSeth commented 1 month ago

@Ocraw would you be willing to help test out a fix for this, since you found this issue and I presume you have an active real-world setup that is affected by this? This was the cause of the issue you mentioned when you took all items, it caused the crafting supplier to take materials as if it had to craft all the items, even though all, except 1, were stored.

Ocraw commented 1 month ago

Yes of course, what do I need to do?

ZenonSeth commented 1 month ago

I don't know how familiar you are with github or the minetest mod structure.

I've pushed what I think is a fix for this on the latest main branch - if you go to the front repo page https://github.com/ZenonSeth/logistica - then click the green Code button, select "Download ZIP". I suggest first backing up the previous /mods/logistica/ by renaming it to like /mods/logistica-orig/ or something - and then extracting the downloaded zip in there and making sure the extracted folder is named logistica

Then test your issue. If it's fine I will publish the changes as a release and then you can delete the extracted logistica folder, rename the original from logistica-orig to logistica and update the usual way you do

Ocraw commented 1 month ago

Usually can move around on github for very basic stuff, but yeah I do use it. Careful updating main for test tho, might want to branch tests off. Trying to be quick:

Fixes this issue, no more materials than reauested are consumed.
Fixes crafting, it uses the correct materials count for one crafting, after stored output is consumed. (still pulls stacked count in recipes but you might already be aware)

ZenonSeth commented 1 month ago

Careful updating main for test tho, might want to branch tests off.

Been using git for almost 10 years in my professional, paying, career - yea branches would normally be how you do stuff, or even forks, but in this case I'm the only dev, and there's no CI that picks up my pushes to main, and releases are manually done by me, so it's not a big deal.

As for fix, cool, if it works correctly, I will try to make a release later today, there's another issue someone reported on the forums I want to look into.

ZenonSeth commented 1 month ago

Release 1.2.3 is published, should fix this, along with some other stuff.

I also started work on https://github.com/ZenonSeth/logistica/issues/18 but I haven't finished it in time to make this release, will have to be sometime later this week.

ZenonSeth commented 1 month ago

Also going to close this as complete, if you encounter it again, please re-open, otherwise feel free to raise new bugs!