minetest-mods / technic

Technic mod for Minetest
Other
146 stars 155 forks source link

frames_1111 crashes when clicked with pipes #643

Closed scr267 closed 2 months ago

scr267 commented 2 months ago

Hi there,

I've upgraded both technic and pipeworks on my server and found that there may be a bug in frames.lua: https://github.com/minetest-mods/technic/blob/80dee96dbe2662d4ec6f389faafaf9890855000e/technic/machines/other/frames.lua#L356

When you place a pipe first, then place a frame on the pipe, everything works well. However, if you place a frame first, then right click on it the server crashes with:

2024-05-18 09:56:22: ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod '' in callback item_OnPlace(): ....minetest/mods/technic/technic/machines/other/frames.lua:356: attempt to call local 'callback' (a nil value)
2024-05-18 09:56:22: ERROR[Main]: stack traceback:
2024-05-18 09:56:22: ERROR[Main]:       ....minetest/mods/technic/technic/machines/other/frames.lua:356: in function 'on_rightclick'
2024-05-18 09:56:22: ERROR[Main]:       /usr/local/share/minetest/builtin/game/item.lua:313: in function </usr/local/share/minetest/builtin/game/item.lua:306>

I've fixed this locally with rubenwardy's help by modifying my code to:

  if callback and callback(pos_copy, newnode_copy, placer, oldnode_copy, itemstack) then
      take_item = false
  end
SmallJoker commented 2 months ago

@za267 The proposed hotfix is not optimal. Please check whether the following change produces the expected result:

diff --git a/technic/machines/other/frames.lua b/technic/machines/other/frames.lua
index c116187..421c3a9 100644
--- a/technic/machines/other/frames.lua
+++ b/technic/machines/other/frames.lua
@@ -324,6 +324,7 @@ for zp = 0, 1 do

                on_rightclick = function(pos, node, placer, itemstack, pointed_thing)
                        if is_supported_node(itemstack:get_name()) then
+                               -- Stripped down version of "core.item_place_node"
                                if minetest.is_protected(pos, placer:get_player_name()) then
                                        minetest.log("action", placer:get_player_name()
                                                .. " tried to place " .. itemstack:get_name()
@@ -347,8 +348,7 @@ for zp = 0, 1 do
                                end

                                -- Run script hook
-                               local callback = nil
-                               for _, _ in ipairs(minetest.registered_on_placenodes) do
+                               for _, callback in ipairs(minetest.registered_on_placenodes) do
                                        -- Copy pos and node because callback can modify them
                                        local pos_copy = { x = pos.x, y = pos.y, z = pos.z }
                                        local newnode_copy = { name = def.name, param1 = 0, param2 = 0 }
scr267 commented 2 months ago

@SmallJoker That does seem to work. Thank you.

SmallJoker commented 2 months ago

f80372a