builtin/item.lua: callbacks with copies of positions and nodes rather than recycle the same ones, which callbacks can modify

1 files changed, 82 insertions, 68 deletions

diff --git a/builtin/item.lua b/builtin/item.lua
index e19fd559a..bd34efe79 100644
--- a/builtin/item.lua
+++ b/builtin/item.lua
@@ -124,76 +124,84 @@ end
 function minetest.item_place_node(itemstack, placer, pointed_thing)
 	local item = itemstack:peek_item()
 	local def = itemstack:get_definition()
-	if def.type == "node" and pointed_thing.type == "node" then
-		local under = pointed_thing.under
-		local oldnode_under = minetest.env:get_node(under)
-		local olddef_under = ItemStack({name=oldnode_under.name}):get_definition()
-		olddef_under = olddef_under or minetest.nodedef_default
-		local above = pointed_thing.above
-		local oldnode_above = minetest.env:get_node(above)
-		local olddef_above = ItemStack({name=oldnode_above.name}):get_definition()
-		olddef_above = olddef_above or minetest.nodedef_default
-
-		if not olddef_above.buildable_to and not olddef_under.buildable_to then
-			minetest.log("info", placer:get_player_name() .. " tried to place"
-				.. " node in invalid position " .. minetest.pos_to_string(above)
-				.. ", replacing " .. oldnode_above.name)
-			return
-		end
+	if def.type ~= "node" or pointed_thing.type ~= "node" then
+		return itemstack
+	end
 
-		-- Place above pointed node
-		local place_to = {x = above.x, y = above.y, z = above.z}
+	local under = pointed_thing.under
+	local oldnode_under = minetest.env:get_node(under)
+	local olddef_under = ItemStack({name=oldnode_under.name}):get_definition()
+	olddef_under = olddef_under or minetest.nodedef_default
+	local above = pointed_thing.above
+	local oldnode_above = minetest.env:get_node(above)
+	local olddef_above = ItemStack({name=oldnode_above.name}):get_definition()
+	olddef_above = olddef_above or minetest.nodedef_default
+
+	if not olddef_above.buildable_to and not olddef_under.buildable_to then
+		minetest.log("info", placer:get_player_name() .. " tried to place"
+			.. " node in invalid position " .. minetest.pos_to_string(above)
+			.. ", replacing " .. oldnode_above.name)
+		return
+	end
 
-		-- If node under is buildable_to, place into it instead (eg. snow)
-		if olddef_under.buildable_to then
-			minetest.log("info", "node under is buildable to")
-			place_to = {x = under.x, y = under.y, z = under.z}
-		end
+	-- Place above pointed node
+	local place_to = {x = above.x, y = above.y, z = above.z}
 
-		minetest.log("action", placer:get_player_name() .. " places node "
-			.. def.name .. " at " .. minetest.pos_to_string(place_to))
-		
-		local oldnode = minetest.env:get_node(place_to)
-		local newnode = {name = def.name, param1 = 0, param2 = 0}
+	-- If node under is buildable_to, place into it instead (eg. snow)
+	if olddef_under.buildable_to then
+		minetest.log("info", "node under is buildable to")
+		place_to = {x = under.x, y = under.y, z = under.z}
+	end
 
-		-- Calculate direction for wall mounted stuff like torches and signs
-		if def.paramtype2 == 'wallmounted' then
+	minetest.log("action", placer:get_player_name() .. " places node "
+		.. def.name .. " at " .. minetest.pos_to_string(place_to))
+	
+	local oldnode = minetest.env:get_node(place_to)
+	local newnode = {name = def.name, param1 = 0, param2 = 0}
+
+	-- Calculate direction for wall mounted stuff like torches and signs
+	if def.paramtype2 == 'wallmounted' then
+		local dir = {
+			x = under.x - above.x,
+			y = under.y - above.y,
+			z = under.z - above.z
+		}
+		newnode.param2 = minetest.dir_to_wallmounted(dir)
+	-- Calculate the direction for furnaces and chests and stuff
+	elseif def.paramtype2 == 'facedir' then
+		local placer_pos = placer:getpos()
+		if placer_pos then
 			local dir = {
-				x = under.x - above.x,
-				y = under.y - above.y,
-				z = under.z - above.z
+				x = above.x - placer_pos.x,
+				y = above.y - placer_pos.y,
+				z = above.z - placer_pos.z
 			}
-			newnode.param2 = minetest.dir_to_wallmounted(dir)
-		-- Calculate the direction for furnaces and chests and stuff
-		elseif def.paramtype2 == 'facedir' then
-			local placer_pos = placer:getpos()
-			if placer_pos then
-				local dir = {
-					x = above.x - placer_pos.x,
-					y = above.y - placer_pos.y,
-					z = above.z - placer_pos.z
-				}
-				newnode.param2 = minetest.dir_to_facedir(dir)
-				minetest.log("action", "facedir: " .. newnode.param2)
-			end
+			newnode.param2 = minetest.dir_to_facedir(dir)
+			minetest.log("action", "facedir: " .. newnode.param2)
 		end
+	end
 
-		-- Add node and update
-		minetest.env:add_node(place_to, newnode)
-
-		-- Run callback
-		if def.after_place_node then
-			def.after_place_node(place_to, placer)
-		end
+	-- Add node and update
+	minetest.env:add_node(place_to, newnode)
 
-		-- Run script hook (deprecated)
-		local _, callback
-		for _, callback in ipairs(minetest.registered_on_placenodes) do
-			callback(place_to, newnode, placer, oldnode)
-		end
+	-- Run callback
+	if def.after_place_node then
+		-- Copy place_to because callback can modify it
+		local place_to_copy = {x=place_to.x, y=place_to.y, z=place_to.z}
+		def.after_place_node(place_to_copy, placer)
+	end
 
-		itemstack:take_item()
+	-- Run script hook
+	local _, callback
+	for _, callback in ipairs(minetest.registered_on_placenodes) do
+		-- Copy pos and node because callback can modify them
+		local place_to_copy = {x=place_to.x, y=place_to.y, z=place_to.z}
+		local newnode_copy = {name=newnode.name, param1=newnode.param1, param2=newnode.param2}
+		local oldnode_copy = {name=oldnode.name, param1=oldnode.param1, param2=oldnode.param2}
+		callback(place_to_copy, newnode_copy, placer, oldnode_copy)
 	end
+
+	itemstack:take_item()
 	return itemstack
 end
 
@@ -243,9 +251,11 @@ function minetest.node_punch(pos, node, puncher)
 	-- Run script hook
 	local _, callback
 	for _, callback in ipairs(minetest.registered_on_punchnodes) do
-		callback(pos, node, puncher)
+		-- Copy pos and node because callback can modify them
+		local pos_copy = {x=pos.x, y=pos.y, z=pos.z}
+		local node_copy = {name=node.name, param1=node.param1, param2=node.param2}
+		callback(pos_copy, node_copy, puncher)
 	end
-
 end
 
 function minetest.node_dig(pos, node, digger)
@@ -268,8 +278,8 @@ function minetest.node_dig(pos, node, digger)
 		local drops = minetest.get_node_drops(node.name, wielded:get_name())
 
 		-- Wear out tool
-		tp = wielded:get_tool_capabilities()
-		dp = minetest.get_dig_params(def.groups, tp)
+		local tp = wielded:get_tool_capabilities()
+		local dp = minetest.get_dig_params(def.groups, tp)
 		wielded:add_wear(dp.wear)
 		digger:set_wielded_item(wielded)
 
@@ -282,10 +292,8 @@ function minetest.node_dig(pos, node, digger)
 		end
 	end
 	
-	local oldnode = nil
 	local oldmetadata = nil
 	if def.after_dig_node then
-		oldnode = node;
 		oldmetadata = minetest.env:get_meta(pos):to_table()
 	end
 
@@ -294,13 +302,19 @@ function minetest.node_dig(pos, node, digger)
 	
 	-- Run callback
 	if def.after_dig_node then
-		def.after_dig_node(pos, oldnode, oldmetadata, digger)
+		-- Copy pos and node because callback can modify them
+		local pos_copy = {x=pos.x, y=pos.y, z=pos.z}
+		local node_copy = {name=node.name, param1=node.param1, param2=node.param2}
+		def.after_dig_node(pos_copy, node_copy, oldmetadata, digger)
 	end
 
-	-- Run script hook (deprecated)
+	-- Run script hook
 	local _, callback
 	for _, callback in ipairs(minetest.registered_on_dignodes) do
-		callback(pos, node, digger)
+		-- Copy pos and node because callback can modify them
+		local pos_copy = {x=pos.x, y=pos.y, z=pos.z}
+		local node_copy = {name=node.name, param1=node.param1, param2=node.param2}
+		callback(pos_copy, node_copy, digger)
 	end
 end