minetest-mods / digtron

A modular tunnel boring/construction machine mod for Minetest.
https://forum.minetest.net/viewtopic.php?f=9&t=16295
Other
22 stars 29 forks source link

Two weird inventory-related issues in multiplayer #19

Open h-v-smacker opened 6 years ago

h-v-smacker commented 6 years ago

With current version of the mod, two weird things occur (both in multiplayer, "Linuxworks Next Generation"):

1: Extraction stops after placement of the first block, and strange_error occurs (the panel dings, reports a "possible error with inventory", and the machine goes on). Using dedicated inventories and running off electricity doesn't help. This happens in protected and unprotected areas alike.

2: When I launch a harvesting digtron (one that harvests food and replants it), the tomatoes in my inventory jump into my hand (active slot) with every digtron move, no matter in which inventory slot I place them myself. Needless to say, the harvester plants and harvests tomatoes.

I have a gut feeling it's somehow related, but I cannot figure out more at the moment.

numberZero commented 6 years ago
  1. Does “current” mean https://github.com/minetest-mods/digtron/commit/33995bc6ecf5b497f4e6b84b72fb476acf203af7?
  2. Which version of farming do you use? Just tested with https://github.com/tenplus1/farming/commit/0843bd0696213358607ac44e886c5e13ccbffebb, ~it works (not cleanly, though).~
  3. What’s the digtron design?
numberZero commented 6 years ago

Just reproduced. Digtorn and farming depend on creative mode too much.

numberZero commented 6 years ago

@FaceDeer Why do you pass the real player to the on_place function?

h-v-smacker commented 6 years ago

@numberZero "current" means the one fetched on this very day.

For version of farming (on the server), gabriel would know better, although I suppose it's also one fetched recently.

The harvester digtron design is an array of downward-facing digger heads for harvesting, followed by 1 block spacing, and then an array of downward-facing builder blocks planting seeds, powered by HV cable. As for extrusion, it's highly irrelevant: I tested a minimal one (panel, combined storage, one builder head) moving/extruding in different directions, it's always the same.

numberZero commented 6 years ago

The reason for 2nd bug is simple: digtron passes real player (instead of fake one) and 1-item stack to tomato’s on_place, which in turn schedules farming.refill_plant (handy feature for manual planting) which does its job well, placing fresh stack of tomatoes in your hand.

gpcf commented 6 years ago

My server is running the newest git version of farming_redo, if this info is of any use.

numberZero commented 6 years ago

You can try applying this patch to farming (safe):

diff --git a/init.lua b/init.lua
index ab700de..d9e0db3 100644
--- a/init.lua
+++ b/init.lua
@@ -454,7 +454,7 @@ function farming.place_seed(itemstack, placer, pointed_thing, plantname)
            itemstack:take_item()

            -- check for refill
-           if itemstack:get_count() == 0 then
+           if itemstack:get_count() == 0 and placer:is_player() and not placer.is_fake_player then

                minetest.after(0.10,
                    farming.refill_plant,

and this to digtron (may break things):

diff --git a/util_item_place_node.lua b/util_item_place_node.lua
index ab78526..ede0268 100644
--- a/util_item_place_node.lua
+++ b/util_item_place_node.lua
@@ -99,7 +99,7 @@ digtron.item_place_node = function(itemstack, placer, place_to, param2)
        -- though note that some mods do "creative_mode" handling within their own on_place methods, which makes it impossible for Digtron
        -- to know what to do in that case - if you're in creative_mode Digtron will place such items but it will think it failed and not
        -- deduct them from inventory no matter what Digtron's settings are. Unfortunate, but not very harmful and I have no workaround.
-       local returnstack, success = def.on_place(ItemStack(itemstack), placer, pointed_thing)
+       local returnstack, success = def.on_place(ItemStack(itemstack), digtron.fake_player, pointed_thing)
        if returnstack and returnstack:get_count() < itemstack:get_count() then success = true end -- some mods neglect to return a success condition
        if success then
            -- Override the param2 value to force it to be what Digtron wants
@@ -147,12 +147,11 @@ digtron.item_place_node = function(itemstack, placer, place_to, param2)

    local take_item = true

-   -- Run callback, using genuine player for per-node definition.
    if def.after_place_node then
        -- Deepcopy place_to and pointed_thing because callback can modify it
        local place_to_copy = {x=place_to.x, y=place_to.y, z=place_to.z}
        local pointed_thing_copy = copy_pointed_thing(pointed_thing)
-       if def.after_place_node(place_to_copy, placer, itemstack,
+       if def.after_place_node(place_to_copy, digtron.fake_player, itemstack,
                pointed_thing_copy) then
            take_item = false
        end

In theory, this should be harmless, as fake_player implements (in some way) the whole ObjectRef interface, and placer is defined to be ObjectRef (or even nil) and not necessarily Player. Nevertheless, many mods assume that it must be a player, thus breaking things.