Minetest_game: Make schematic placed chests work

[MarkuBu]

If chests are placed by a schematic they don't work. If you open a chest, no inventory is shown. This is a simple solution to fix that

        def.on_rightclick = function(pos, node, clicker)
            local meta = minetest.get_meta(pos)
            if meta:get_string("infotext") ~= "Chest" then
                minetest.registered_nodes["default:chest"].on_construct(pos)
            end

Add this code to the on_rightclick callback (similar for locked chest)

[sfan5]

it would be a cleaner solution to just do it properly when placing the schematic in question

[MarkuBu]

I don't understand what you mean

[paramat]

This also happens if placing chests with the decoration code or with the lua voxel manipulator.
I'm guessing sfan5 means that when a mod places a chest by any of these methods it should then set the metadata for the placed chests?

[MarkuBu]

How should that work? If I place a building, should I use find_nodes_near to search for the chests? Or use LBM/ABM to add metadata?

I think, my solution is easier and more efficient. If a chest is placed as schematic it has no metadata, including infotext. Just check for metadata and call the function, if it's missing.

With a little modification mods would be able to add some loot without the need to override something or edit nodes.lua

[paramat]

Yes i think your idea is potentially good as a fallback or a simple method for when a modder just wants empty chests placed, it also moves the setting of metadata from schematic placement / mapgen time to some random later time, which is useful as it reduces load at schematic generation / mapgen time.

[MarkuBu]

The only "disadvantage" of this method is, that chests have no infotext until they are initialised

And btw, bookshelf and furnace have the same problem. Want to find a similar solution

[paramat]

Best to not use 'find nodes near' or ABM, but of course, the random rotation of schematics means we don't know where a chest will be placed by a schematic, so your suggestion actually seems the simplest and most lightweight method.

chests have no infotext until they are initialised

Very minor issue, players can see it is a chest and will open it.

[paramat]

So i will support this, unless i missed some reason why it is bad.

In the case of initialising the chest and placing items in it, if the schematic is not randomly rotated you will know where the chest will be by looking at it's position in the schematic.
If the schematic is randomly rotated then i guess 'find nodes in area' is the thing to use. ABMs should be avoided if possible.

[paramat]

Author no longer makes PRs so needs adoption.

[paramat]

@MarkuBu if you find code that does the same for bookshelves and furnaces please paste it here for inclusion into a PR. I might make a PR for all.

[MarkuBu]

I'm working on it

[Desour]

Imo it's not good to check the infotext for this, there are mods that allow players to modify infotexts and co.

[MarkuBu]

Should be enough to check if the infotext already exists. Schematic placed chests don't have an infotext

[paramat]

I'll probably add this myself, it's a good idea.

[Wuzzy2]

Sorry, this is just hacky. It would be better if the chest is initialized after being placed by a schematic.
By the way, I don't like hardcoding the chest positions, nor do I like to do find_nodes_in_area because Minetest already knows the chest position (but it throws it away).

Sadly, there doesn't seem to be a clean way to grab the chest positions after schematic placement, except for scanning all nodes again.
Why do you ignore sfan5? :O This concern is valid.

There's another problem. Any nodes wanting to interact (especially from mods) with an uninitialize chest will fail until the chest is rightclicked. That's why I said this is hacky.

Examples: Hoppers (transfer items to chests/hoppers downwards), vending machines (placed on top of chests to exchange items with chest). Mods must be able to safely assume that chests have an inventory.

Oh, also this idea just bad because it is not generalized. You totally forgot bookshelves, vessels shelves, furnaces and whatever else needs on_construct.

This “initialize nodes after being placed by a schematic” thing is so essential, it should be fixed in the engine. Asking modders to manually fix on_rightclick for all their nodes is horrible.

[MarkuBu]

You are right, if nodes are placed by schematic the 'on_construct' function should be called. But I'm not sure how this will effect performance.

Maybe a schematic should have a flag 'call_on_construct' and if it is true 'on_construct' will be called for each node.

Or a lua schematic can have an 'on_construct' callback to initialise after a schematic is placed (and maybe even a 'before_construct' callback). This could be used for more complex and rarely used structures. I don't think, this would have a noticeable impact to the performance

[paramat]

It would be better if the chest is initialized after being placed by a schematic.

True, but not by schematic code.

There's another problem. Any nodes wanting to interact (especially from mods) with an uninitialize chest will fail until the chest is rightclicked. That's why I said this is hacky.

Good point.

[paramat]

The only issue is when random rotation is used, otherwise you know where nodes are.
Maybe rotation could be returned by 'place schematic'? (not for those placed by decoration API though).

[paramat]

Would it be enough to just check for the existence of metadata? If none, call 'on construct'?

[Ferk]

Maybe I'm wrong, but I imagine that since both nodedef and metadata are already sent to the client, the formspec in the metadata is loaded client-side and the server probably doesn't really need to do anything at all, so the formspec shows instantly whenever it's a metadata formspec vs one loaded by on_rightclick.

Having an on_rightclick callback would require communication with the server before the formspec is even displayed to the user, so it's affected by possible network lag + server processing if it needs to check the metadata and possibly write it.

EDIT: ow... sorry, I missed this change: https://github.com/minetest/minetest/pull/6232 So in the latest version it's possible to have both form metadata and on_rightclick at the same time. This means there would only be lag when showing the formspec the first time the chest is opened, the following times I assume the client will open the formspec instantly and later the server will just check the metadata and do nothing.

[paramat]

The initial suggestion is problematic, and initialising these nodes should be done by the mod, it's not the job of game or engine to do this :-1: