Add methods for checking whether multiple stacks can fit into or be extracted from an inventory

[andriyndev]

This PR adds two methods for InvRef:

• room_for_items(listname, stacks_array)
• contains_items(listname, stacks_array, [match_meta])

These methods are similar to room_for_item(listname, stack) and contains_item(listname, stack, [match_meta]) except that they perform the same check for several stacks instead of only one.
Using the new functions is not the same as calling the existing (single-stack) functions for each stack separately, because in the latter case it'll check whether each separate stack can be added to the inventory without the other stacks, while the new functions check whether all the items from the list can be added to or removed from the inventory together.
This feature is useful when a mod needs to check whether a set of stacks can be fully extracted from or fully placed to an inventory.
Without this patch, there is no a simple way to do the same. Possible solutions:

• Implement an algorithm inside the mod which will perform the corresponding checks with the existing API. It'll be quite difficult, and easy to do incorrectly.
• Use a third-party mod which provides the corresponding functionality. It'll introduce an additional dependency.
• Create a detached inventory and perform the simulation on it. It looks like overkill.

To do

This PR is a Work in Progress

• Add comments
• Add information to the documentation
• Check the correct return value when all passed itemstacks are empty
• Test with different combinations, add unit tests
• Possibly move duplicated code into separate functions
• Add "How to test"

How to test

1. Install the mod with the following init.lua: init.txt
2. Place the node "Node with formspec" from this mod
3. Right-click the node. There will be three inventories: node's main inventory, player's main inventory, and the inventory which denotes which items to move. Also, there will be two button: one - for moving the items from player's to node's inventory, and the second - for moving the items from node's to player's inventory.
4. Move items to the third inventory which will display the itemstacks which will be moved. Press the buttons to try to move the itemstacks between inventories.
   Before moving items between inventories, the two methods introduced in this patch will be called. If one of them returns false, the corresponding message will be printed into log. If the both functions returned true, but moving items between inventories actually failed because of lack of items or space, it'll print a log message with "AAAAAAAAAAAAAAAAAAA", which might indicate a bug in the newly implemented functions. Since the current wersion of remove_item() doesn't take metadata into account, the test uses a separate function which should do this.

[appgurueu]

Create a detached inventory and perform the simulation on it. It looks like overkill.

I agree that this would be a rather dirty and wasteful hack.

But I think it hints at what at a proper solution should look like.

Essentially, I think we want a proper "inventory list" data structure. This data structure should be copyable. Then you would simply get an inventory list, copy it, try to add / take the items, and if it worked, swap the original for the mutated copy.

Use a third-party mod which provides the corresponding functionality. It'll introduce an additional dependency.

Not much of a problem. Also: If you already depend on a mod for other library functions, this won't necessarily introduce an additional dependency.

This argument is a very slippery slope: By this logic, Luanti would essentially have to become the union of all library mods on ContentDB, and the concept of a library mod should cease to exist.

Note also that you could "bundle" libraries with your mod rather than having them be separate mods to avoid additional dependencies.

Implement an algorithm inside the mod which will perform the corresponding checks with the existing API. It'll be quite difficult, and easy to do incorrectly.

Implement it once, write some good unit tests, put it in a library mod.

It's in many ways worse if we do the equivalent via Luanti's C++ codebase:

• The C++ code is likely to be more work to maintain: Lua is more expressive and avoids entire classes of errors. You don't seem to be gaining much by skipping the Lua API. Even if this was to be done in the engine, I would frankly doubt whether it needs to be in C++. A pure Lua builtin utility seems like a better option. (I suppose opinions may vary a bit depending on static vs dynamic typing preferences, but I think few consider C++ more maintainable for such a well-constrained problem in any case.)
• It being C++ also makes it harder for modders to understand or tweak / fork the code.
• Things will take a while. You could have your library mod published on ContentDB in a couple days. Even if you were to quickly get two core dev approvals on this PR (and generally there will be some back-and-forth and bikeshedding), you would still have to wait till the next point release until it will realistically be available for anyone, and even then will it not be available for older versions (which will still be in widespread use for a while) for no reason. A library mod can make this available very soon, without a need for coredev approval, for all 5.x versions.
• Luanti now becomes the bottleneck for everyone wanting to do something with this code - tweaks, bugs, feature requests, whatever, it all has to go through us.

Conclusion

I would strongly recommend doing this in a library mod first instead. Once/if that library mod sees widespread adoption, moving it to the engine can still be reconsidered. (In general, this is a path I'd like to become more standard for "convenience" features that can be implemented without issue in a library mod.)

I think this PR is, as it stands, low priority and should potentially be rejected, I'm afraid.

[andriyndev]

@appgurueu thank you for your opinion!

Even if this was to be done in the engine, I would frankly doubt whether it needs to be in C++. A pure Lua builtin utility seems like a better option.

As I see, all the InvRef methods in the engine are implemented in C++. And ItemStack, as far as I remember, is a userdata object.

Things will take a while.

Currently, my mod already have a set of functions, which do the same, but, I think, it'd be great to have this functionality for future generation of modders. Especially taking into account that there were problems with this on the server I'm playing on, when items were disappearing because there wasn't enough place for all of them in the inventory, because the mechanism of checking this in that mod was buggy (maybe there were more than one mod with this problem but don't remember).

Implement it once, write some good unit tests, put it in a library mod.

I could do this, but other mod developers who need this functionality will need to do the same, i.e. implement and test it. They could, in theory, re-use my library, or I could also reuse someone else, but if my mod will depend on a third-party library, its creator should be trustworthy enough, otherwise he/she might just break my mod by a buggy patch, or if I just copy the code into my mod (if its license allows it), in case of possible fix in the upstream I might not know about it, and the bug will remain in my code.

My arguments might be exaggerated, but taking into account that the feature might be demanded by different mods, and it might be difficult to implement correctly, I think, it'd be great to have it "out of the box".

[cx384]

This feature seems quite useful to me, but I agree with appgurueu.
(Maybe we also want add_items and remove_items.)

It's only a convenience feature, so it is fine to make it low priority.
If you are desperate to have this feature in the near future, I suggest to implement it as a Lua library using the result of get_list(listname).

[andriyndev]

Maybe we also want add_items and remove_items.

I think, adding these functions won't make much sense, since we can easily do the same by subsequent calls to :add_item() and :remove_item() in a loop for each stack separately. The only advantage of these functions would be a bit more efficiency since we won't need to iterate over the inventory for each stack from the beginning, but since it's usually done not very often, it will hardly affect the overall performance at all (pipeworks' tubes aren't counted since they operate by separate stacks anyway). In contrast, the functions implemented in this patch will require a much more complex algorithm, especially if metadata matters, and it's quite easy to do it incorrectly, which might have quite serious consequences, like losing items (especially rare), or getting them out of air up to infinity, or turning an item with one metadata into an item with another one (like Empty bag -> Bag with rare items).

If you are desperate to have this feature in the near future, I suggest to implement it as a Lua library using the result of get_list(listname).

That's what I already did. Moreover, my implementation also calculates the number of available extractions. The current patch is intended for future "generations" of mod developers, so that they won't need to implement the algorithm manually with all the risks, and just use the functions out of the box.

[andriyndev]

Essentially, I think we want a proper "inventory list" data structure. This data structure should be copyable. Then you would simply get an inventory list, copy it, try to add / take the items, and if it worked, swap the original for the mutated copy.

It's an interesting idea. However, we already have a set of functions for working with inventory, which don't use such object, and can perform most of basic operations. Adding such object will require adding a set of dedicated functions which will mostly duplicate already existing functions, which don't use such data structures.