[Mod] Hoppers [hopper]

[TenPlus1]

Thanks for your ideas FaceDeer but for now I'm going to keep hoppers simple and fast...

Updated to 1.3:

- Tweaked code to use single abm, performance increase.
- Added intllib support, german language conversion by Xanthin
- Side-hopper inventory items can be crafted back into standard hoppers

Download: https://github.com/tenplus1/hopper/archive/master.zip

[texmex]

There's a "reverse hopper" item with this mod, I believe it could be useful to possibly merge the concept into the more established hopper mod.

[FaceDeer]

There's a "reverse hopper" item with this mod, I believe it could be useful to possibly merge the concept into the more established hopper mod.

I've taken a stab at this with the minetest-mods branch of this mod, I added an "eject items"/"don't eject" button to the inventory formspecs that you can use to set a hopper to eject items out into the "real world" if there's no suitable inventory to receive them. Let me know if this is what you were thinking of.

[texmex]

I've taken a stab at this with the minetest-mods branch of this mod, I added an "eject items"/"don't eject" button to the inventory formspecs that you can use to set a hopper to eject items out into the "real world" if there's no suitable inventory to receive them. Let me know if this is what you were thinking of.

Awesome. No time to test the changes right now but here's what I think.

For me the most intuitive usage flow would be for hoppers to become ejectors whenever they're placed (or screwdrived) upside down, connecting to a chest, furnace, regular hopper or chute. Like so:

[FaceDeer]

For me the most intuitive usage flow would be for hoppers to become ejectors whenever they're placed (or screwdrived) upside down, connecting to a chest, furnace, regular hopper or chute.

I'm kind of the opposite on that - it seems unintuitive to me to have the direction of item flow through hoppers, and their overall behaviour, change based on its orientation. Right now items always go from wide end to narrow end, making it straightforward to trace the path items will take with a visual glance. They're like big three-dimensional arrowheads.

I'd also like to make it hard to accidentally rig up a hopper to eject items out into the world. Items sitting in the world as entities can easily be lost for good, whereas a hopper that's merely sticking stuff into the wrong inventory will at worst get clogged up and sit there quiescently until a user comes along to clear the jam and correct the problem.

Give the toggle-button version I've implemented a shot, I hope it will satisfy. If only because I dread the complexity of trying to code up the alternative (the toggle button version was quite easy to hook into the existing methods :)

[Wuzzy]

I haven't looked at this mod for a while, but for MineClone 2 I use this mod, but heavily modified it for my needs. In MineClone 2, the hoppers already interact with chests, furnaces, shulker boxes, dispensers, other hoppers and droppers, without actually depending on any of these. I have also heavily refactored the code to remove tons of code duplication which was an endless source of bugs (in MineClone 2).

I'm just writing this here because I think it might interest you.

I can give you some insights on what I have done:
I defined a group “container” which is defined like this:

container=<any value>: For nodes with an inventory
container=2: Node with a singe inventory list, called “main”
container=3: Same as above, but does not allow shulker boxes to be stored inside (shulker boxes are a special kind of chest in MineClone 2 / Minecraft)
container=4: For furnaces, it must have 3 inventory lists: src, dst and fuel (like in Minetest Game).

Chests, hoppers, dispensers and droppers (these are all nodes with a simple chest-like inventory) have the group “container=2”.
Shulker boxes are in container=3.
Furnaces are of course in container=4.

Now the hopper basically just looks at the group of the node and not at the node itself. The hopper does not depend on chests, shulker boxes, or furnaces or any particular node for its operation. Because it knows how the nodes behave based on their group and what the names of the inventory lists are, it can act accordingly. It is trivially easy to add hopper support for a custom chest from a mod. Just make sure you only use the inventory list “main”, add container=2 to the groups list, done. Not even a soft dependency is required. :-)

Now this works for MineClone 2 and mods for MineClone 2 but not for the rest because this is not a standard.

I think we, the modders, should REALLY establish some sort of “standard groups”, that is, we agree on a set of useful and clearly-defined groups which are generic enough that they can be used without problems in any mod or subgame. The container group above could be something like that, but this is off-topic now.

I hope you find my insights useful. I'm not suggesting you to do anything, just giving you an explanation on what I've done with your mod so far. :-)

[FaceDeer]

I think it may be both possible and a good idea to support both approaches. Off the top of my head, it seems like it'd be pretty straightforward to add the ability to use the Hopper API to register hopper behaviour for a node group instead of a specific node name. I'm going to be out late tonight but I'll take a look at the code when I get home and see how it goes.

Having different hopper behaviours for different node group *values* might be a bit trickier, but I'll see how that goes. Might be workable.

Maybe once I'm done the mod will be suitable for use by your subgame out-of-the-box. I'll see if I can swing that. I've heavily refactored the code myself with an eye toward making it as efficient and general-purpose as possible, so I'd love the opportunity to test it out "in the field" like that.

[sofar]

I'd argue that a callback type implementation may help solve this problem in an easier way than groups. Groups still mean that every node has to determine how to handle the items (reject? move to secondary list? eject?)

A callback would solve this problem - every node could just call the call back of the "other" node and if the return values contain the modified itemstack, everything would be possible without knowing anything about the node or even writing generic code in the first place.

e.g. if the itemstack is returned verbatim, the item is should be put back or ejected, depending on what makes sense for the "giver"
if the itemstack is empty on return, it was accepted. Maybe the other block ejected it
maybe a different item was given back.

direction may be relevant, so a callback should involve a `dir` vector to signal the directionality.

`item_handoff(otherpos, dir, itemstack)` or something like that.

(yes yes I know wuzzy is going to complain about the complexity of this, he generally dislikes interfaces like these, but I think they're better suited than overloading the group bitfields since they allow arbitrary (and yet-to-be-written) code to deal with any situation.)

[Wuzzy]

I still think having defined groups for containers is useful in general. There are use cases which go beyond hoppers. Maybe all a mod wants to know if there are chest-like nodes nearby, and just read the inventory contents, not move it. Or for vending machines above chests. It is soooo annoying that you have to depend on 1000 chest mods only to do the same thing for all mods.
I intended these groups mostly for informational purposes. Like “hey! I'm a chest! I have an inventory list called ‘main’.”. How mods deal with this information is their business. Item transfer with hoppers is just one use case.
These groups all have defined underlying assumptions. E.g. “container=2” also means you can freely take and put items into it. So Minetest Game's bones are already excluded.

I don't really understand your callback approach, to be honest. But if you say that every chest-like node now must supply its own callback functions, this is clearly the wrong way. There are many many nodes in Minetest mods which are basically just chest variants which behave like the Minetest Game chest. And the callback code would essentially be the same for all chests.

But maybe I just misunderstood you a bit. Because I am not against callbacks as such. I also have to admit that my hoppers mod does not work solely work on groups. It makes use of an utility mod which I also wrote in order to handle item transfer. But it's not so bad, I think.
At the end of the day, there's still some code needed somewhere. I know from experience that my approach works just fine for my subgame.
Or did you maybe mean new callbacks in the Lua API? That would be interesting, and I would like to read more about it.

But now I can't really comment on your callback approach since I have no real idea on how exactly it is supposed to work, let alone how it works in practice. But thanks for your thoughtwork so far. :-)

EDIT:

Code: Select all

Maybe once I'm done the mod will be suitable for use by your subgame out-of-the-box. I'll see if I can swing that. I've heavily refactored the code myself with an eye toward making it as efficient and general-purpose as possible, so I'd love the opportunity to test it out "in the field" like that.

Don't bother trying to tailor your mod directly towards MineClone 2. It would be pointless, MineClone 2 already has working hoppers and you would end up just repeating what I already did. I would be unlikely I include it.
This is just a heads-up for you. Do what you mod what you have to do, but I don't guarantee a re-inclusion into MineClone 2 at all.

[FaceDeer]

I still think having defined groups for containers is useful in general.

Oh, certainly, and I agree. I think our approaches may ultimately be completely compatible, to the benefit of both.

I don't really understand your callback approach, to be honest.

Much of it's not my callback approach per se, my fork of Hopper is hooking into the existing callbacks that are defined in the basic Minetest lua API.

I'll describe an overview of how my version of Hopper works under the hood, and how I think it might integrate with your approach (bearing in mind that I still haven't actually looked at your subgame's code yet, I just got in the door after my Friday night RPG session. :)

First, Hopper now maintains a table that tells its item-transfer ABM which nodes have what inventories and in what ways they're accessible. That table gets entries added to it via a public API call that the Hopper mod exposes for other mods to use.

As a concrete example, I just yesterday hooked up the "workbench" node from the castle mod to be hopper-compatible. The workbench has two inventories, "src" (where raw materials are placed) and "dst" (where finished products are put after the workbench auto-crafts things from the raw materials in "src"). I wanted hoppers that injected stuff into the top and sides to inject into the "src" inventories and hoppers drawing from the bottom of the workbench to take stuff from the "dst" inventory. So I added this:

Code: Select all

if minetest.get_modpath("hopper") and hopper ~= nil and hopper.add_container ~= nil then
	hopper:add_container({
		{"top", "castle:workbench", "dst"},
		{"side", "castle:workbench", "src"},
		{"bottom", "castle:workbench", "src"},
	})
end

(the extra ~=nil checks are because earlier versions of hopper don't have this API, I don't want to crash if someone's got an old version of hopper installed).

That's all that's necessary on the part of the other mod, hopper handles everything else after that through standard Minetest API calls. The Hopper ABM detects what nodes are adjacent to a hopper and looks up what inventories are being interacted with from the information provided by this call.

Then once the ABM knows what node and inventory name it's interacting with, it uses the allow_metadata_inventory_put and allow_metadata_inventory_take methods defined by that node's node def to determine whether the action it's trying to do is allowed. This method is what furnaces use to disallow any non-fuel items from being put into the fuel slot, for example, and what locked chests use to prevent non-owners from taking items out of it.

If the item transfer is permitted, the hopper goes ahead and does it. Then it calls the respective on_metadata_inventory_put and on_metadata_inventory_take methods from the target node's definition to let the node know that the transfer took place. This is where the furnace checks to see whether it should "ignite" itself when stuff gets dropped into its src and fuel inventories, for example. These methods are also part of the standard Minetest API and are used when the player inserts or takes things from inventories so they can generally be relied upon.

What I'm proposing, based on your description of what you want out of group-based hopper actions, would be something like this:

Code: Select all

if minetest.get_modpath("hopper") and hopper ~= nil and hopper.add_container ~= nil then
	hopper:add_container({
		{"top", "group:container", "main"},
		{"side", "group:container", "main"},
		{"bottom", "group:container", "main"},
	})
end

which would then cause hopper to treat all nodes belonging to group "container" as having a "main" inventory to insert into and draw from. Or use "{"top", "group:container=2", "main"}" to be specific as to the group value to treat like that.

You'd only need to put that hopper:add_container call into a subgame once, and then any node that belongs to those groups would be handled by hoppers without having to declare a dependency.

EDIT:

Code: Select all

Maybe once I'm done the mod will be suitable for use by your subgame out-of-the-box. I'll see if I can swing that. I've heavily refactored the code myself with an eye toward making it as efficient and general-purpose as possible, so I'd love the opportunity to test it out "in the field" like that.

Don't bother trying to tailor your mod directly towards MineClone 2. It would be pointless, MineClone 2 already has working hoppers and you would end up just repeating what I already did. I would be unlikely I include it.
This is just a heads-up for you. Do what you mod what you have to do, but I don't guarantee a re-inclusion into MineClone 2 at all.

if I do this right then I won't have to tailor hopper directly towards MineClone 2. :)

Edit: I couldn't sleep, so I've gone ahead and made the changes described above to the minetest-mods fork of hopper. While I was in there I also made the "default" mod dependency into an optional dependency, though without the default mod hopper no longer defines its own crafting recipes. Also the formspecs might be a little bland without the default background image. I can add something to the API for inserting stuff into the formspec if you like.

[FaceDeer]

I've made an update to the minetest-mods hopper version, adding a new type of block: a "sorter" node. This is essentially a chute node with an alternate output that you can use to shunt particular items to a different destination.

There is a set of inventory slots labeled "Filter" at the top of this block's inventory display, if you place an item into one of these slots the sorter will record the item's type (without actually taking it from you). Then when items come through the sorter's inventory that match one of the items in the filter list it will first attempt to send it in the direction marked with an arrow on the sorter's sides. If the item doesn't match the filter list, or the secondary output is unable to take the item for whatever reason, the sorter will try to send the item out the other output instead.

Credit goes to Burli for prompting me to do this with his Hopper with hopper sorter fork. I wound up rewriting the sorter entirely when I implemented it here (Burli forked off of the original hopper, which my fork has diverged too far from at this point for easy merging) so my version behaves a bit differently.

[Byakuren]

I don't really understand your callback approach, to be honest. But if you say that every chest-like node now must supply its own callback functions, this is clearly the wrong way. There are many many nodes in Minetest mods which are basically just chest variants which behave like the Minetest Game chest. And the callback code would essentially be the same for all chests.

Currently you set the group for all "chest-like" containers to 2. You could instead have a pre-made function that you reuse as a callback in every chest-like container, and expose this function so other mods can use it for their chest-like containers as well. You could even give it custom parameters like item list name.

Example:

Code: Select all

function blah.make_chest_callback(listname)
  return function(pos, itemstack, source_pos)
    -- Put the stack in the specified item list
  end
end

blah.generic_chest_callback = blah.make_chest_callback("main")

minetest.register_node("blah:chest", {
  -- Other fields
  hopper_callback = blah.generic_chest_callback,
})

It wouldn't make it any more difficult than it is now, except that you move the chest-insertion code into a callback. If you wanted to keep the search capability, though, you could have groups in addition to the callback. There's no reason you can't have both.

For groups I would just use a separate group for each kind of container, in addition to the generic group for all containers. Every container would be both in the generic group, but also in the specialized group. This lets a mod look for both containers in general and specific subsets of containers, depending on what it wants to do. It also prevents collisions between different container type group numbers that might happen in Mineclone 2 if a mod adds a novel kind of container.

Example:

Code: Select all

minetest.register_node("blah:furnace", {
  -- Other stuff
  groups = {
    container = 1,
    furnace = 1,
  },
})

minetest.register_node("blah:chest", {
  -- Other stuff
  groups = {
    container = 1,
    chest = 1,
  },
})

[texmex]

Not relating to the immediate discussion in this thread, but I will try the sorter node out later because the new feature sounds great. A dream situation would be for it to work in conjunction with the freshly released drawers mod, don't you think?

[FaceDeer]

Not relating to the immediate discussion in this thread, but I will try the sorter node out later because the new feature sounds great. A dream situation would be for it to work in conjunction with the freshly released drawers mod, don't you think?

Just had a look through drawer's code and compatibility will require a bit of work. A drawers node doesn't actually have an inventory, when you "put something into it" it records the item's name and quantity as metadata values. So none of the standard inventory add/remove callbacks exist, I'll have to add them to the mod and submit a pull request.

I'm liking the concept of drawers, it's very elegant from a player's perspective. Would have loved to have something like this back when I was playing instead of modding. :)

[Byakuren]

Not relating to the immediate discussion in this thread, but I will try the sorter node out later because the new feature sounds great. A dream situation would be for it to work in conjunction with the freshly released drawers mod, don't you think?

Just had a look through drawer's code and compatibility will require a bit of work. A drawers node doesn't actually have an inventory, when you "put something into it" it records the item's name and quantity as metadata values. So none of the standard inventory add/remove callbacks exist, I'll have to add them to the mod and submit a pull request.

I'm liking the concept of drawers, it's very elegant from a player's perspective. Would have loved to have something like this back when I was playing instead of modding. :)

Maybe you can allow the user to optionally specify a function instead of an inventory list name when defining a container? If you want to both take and put items, you would need to take two functions, though.

[FaceDeer]

Maybe you can allow the user to optionally specify a function instead of an inventory list name when defining a container? If you want to both take and put items, you would need to take two functions, though.

If I can update drawers to make use of the standard inventory put/take API then that will allow it to more easily interact with any mod that deals in those concepts, not just hoppers. I'd much prefer going that route if I can, even if it's a bit more work up front.

I'll have some time after work today to see what I can do. I've got some ideas just from browsing the code.

[Byakuren]

The "standard inventory put/take API" expects a valid player for one of its arguments, so it's not really a solution for automated insertion/taking. And anyway, there's no reason the callbacks I described would have to be limited to hopper use, it could be used for any insertion/extraction of items.

[FaceDeer]

My version of hoppers keeps track of the player who placed the hopper in question and passes that in the parameters for the inventory callbacks, this allows them to make appropriate use of locked chests and other protected inventories.

Sure, the new callbacks could be used by things other than hoppers. But it's better to avoid the proliferation of new APIs and interfaces when there are existing ones covering this use case that are documented in the main lua_api.txt and that every modder knows already.

[Byakuren]

My version of hoppers keeps track of the player who placed the hopper in question and passes that in the parameters for the inventory callbacks, this allows them to make appropriate use of locked chests and other protected inventories.

Sure, the new callbacks could be used by things other than hoppers. But it's better to avoid the proliferation of new APIs and interfaces when there are existing ones covering this use case that are documented in the main lua_api.txt and that every modder knows already.

What do you mean keeps track of the player? Keeps the player object around (by saving it in an upvalue or table field or somewhere)? You will have issues if the player logs out or their object is otherwise invalidated. You could limit use to when the player is online, but wouldn't that be too limiting?

[FaceDeer]

What do you mean keeps track of the player? Keeps the player object around (by saving it in an upvalue or table field or somewhere)? You will have issues if the player logs out or their object is otherwise invalidated. You could limit use to when the player is online, but wouldn't that be too limiting?

Well, this is an unhappy revelation. I was recording the player's name and using get_player_by_name to retrieve the player object when needed, but you sparked sufficient suspicion here that I just did some additional testing and it appears that get_player_by_name only returns an object if the player's logged in. That's not documented in lua_api.txt, you'd think that would be kind of an important detail.

I may have to do some special-case coding for this after all. Or perhaps cache the results of the permission check for when the player is offline, or add a new API call as suggested. How annoying, everything would have worked so elegantly were it not for that omission. At least I wrote the code to fail gracefully on an unexpected nil.

In the meantime, I added a feature to the sorter this morning; a button to disable the filter and turn it into an "overflow" handler instead. When a sorter's filter is disabled it will always first attempt to shunt an item into the filter path (indicated by the big arrow) and if that fails (for example due to the target inventory being full) it will fall back to sending it down the other path. Should be handy when filling an array of chests with a huge amount of material, or when filling selective things like furnaces or drawers.

[Byakuren]

I really wish inventories had callbacks that were called for both player and non-player interactions. Maybe something like that could be integrated into the current API in the engine as something alongside the player-specific ones (which might be deprecated).

If hoppers implements its own API then we will have two inventory-moving callback APIs, since pipeworks already has one. Maybe hoppers can use the pipeworks API? Mods don't have to depend on pipeworks to set pipeworks callbacks.

EDIT: Actually, I think inventories shouldn't even need to be the current kind of inventories. Just something with the inventory methods. If callbacks could intercept insertions though, this would basically be fulfilled. Detached inventories can use upvalues to maintain state, and nodes can use node metadata.

[FaceDeer]

I really wish inventories had callbacks that were called for both player and non-player interactions.

Some standards are good, so more must be better I guess. :)

I've worked with Pipeworks compatibility in the past and it wasn't too bad, I'll see if I can have hoppers take advantage of existing Pipeworks interfaces when they already exist. That'll simplify things for implementers somewhat. Maybe I can even have hoppers automatically register pipeworks-enabled nodes. And I can provide "template" functions for implementers to make use of that should easily cover a lot of the basic cases - the owner check for locked chests just requires comparing the placer's name to the owner's name, for example, a simple string comparison between two metadata values.

This is doable. It's just annoying that it needs to be done (and that the documentation misled me into thinking it didn't). The existing API was so close to handling it.

[Byakuren]

I really wish inventories had callbacks that were called for both player and non-player interactions.

Some standards are good, so more must be better I guess. :)

I've worked with Pipeworks compatibility in the past and it wasn't too bad, I'll see if I can have hoppers take advantage of existing Pipeworks interfaces when they already exist. That'll simplify things for implementers somewhat. Maybe I can even have hoppers automatically register pipeworks-enabled nodes. And I can provide "template" functions for implementers to make use of that should easily cover a lot of the basic cases - the owner check for locked chests just requires comparing the placer's name to the owner's name, for example, a simple string comparison between two metadata values.

This is doable. It's just annoying that it needs to be done (and that the documentation misled me into thinking it didn't). The existing API was so close to handling it.

I wish people would stop posting this xkcd comic when it barely applies. A better API in the engine would prevent proliferation of similar APIs.

[FaceDeer]

I wish people would stop posting this xkcd comic when it barely applies. A better API in the engine would prevent proliferation of similar APIs.

But we don't have that better engine API, so proliferation ahoy it seems. I think that makes the comic applicable here. I don't want to add custom stuff, and will strive to avoid it if at all possible, but it now looks impossible in this situation.

I'll spend my evening on this project and see what I can manage.

[Byakuren]

I wish people would stop posting this xkcd comic when it barely applies. A better API in the engine would prevent proliferation of similar APIs.

But we don't have that better engine API, so proliferation ahoy it seems. I think that makes the comic applicable here. I don't want to add custom stuff, and will strive to avoid it if at all possible, but it now looks impossible in this situation.

I'll spend my evening on this project and see what I can manage.

I was speculating on adding a better engine API in the post you responded with XKCD to.