fixes as per code review completed (2)

game_api.txt

@@ -85,11 +85,9 @@ e.g. `table.insert(bones.player_inventory_lists, "backpack")`
 
 Additionally, callbacks can be registered to transfer items into the bones on death:
 
-`bones.register_dead_player_inv_management(function(player,callback){})`
+`bones.register_dead_player_inv_management(function(player){})`
 
-In the above functions the provided callback should be used to add items to the bones inventory:
+the registered function is supposed to return with a table of items to be transferred.
-
-`callback(stack)`
 
 please note that the inventory these items were taken from still need to be disposed of.
 

mods/bones/init.lua

@@ -6,7 +6,7 @@
 -- Load support for MT game translation.
 local S = minetest.get_translator("bones")
 
-local theoretical_max_slots = minetest.settings:get("bones_max_slots") or ( 15 * 10 )
+local theoretical_max_slots = minetest.settings:get("bones_max_slots") or 15 * 10
 local dead_player_callbacks={}
 
 bones = {}
@@ -208,66 +208,34 @@ function bones.register_dead_player_inv_management(func)
 	table.insert(dead_player_callbacks, func)
 end
 
-local function transfer_stack_to_bones(stk,current_dead_player)
+local function player_dies_transfer_inventory(player)
-	-- check if it's possible to place bones, if not find space near player
+	local result = {}
-	if ( current_dead_player.bones_mode == "bones" ) and
-			( current_dead_player.bones_pos == nil ) then
-		current_dead_player.bones_pos = current_dead_player.player_pos
-		local air
-		if may_replace(current_dead_player.bones_pos, current_dead_player.player) then
-			air = current_dead_player.bones_pos
-		else
-			air = minetest.find_node_near(current_dead_player.bones_pos, 1, {"air"})
-		end
-
-		if air and not minetest.is_protected(air, current_dead_player.player_name) then
-			current_dead_player.bones_pos = air
-			local param2 = minetest.dir_to_facedir(current_dead_player.player:get_look_dir())
-			minetest.set_node(current_dead_player.bones_pos, {name = "bones:bones", param2 = param2})
-			local meta = minetest.get_meta(current_dead_player.bones_pos)
-			current_dead_player.bones_inv = meta:get_inventory()
-			--make it so big that anything reasonable will for sure fit inside
-			current_dead_player.bones_inv:set_size("main", theoretical_max_slots)
-		else
-			current_dead_player.bones_mode = "drop"
-			current_dead_player.bones_pos = nil
-		end
-	end
-
-	if ( current_dead_player.bones_mode == "bones" ) and
-			( current_dead_player.bones_inv:room_for_item("main", stk) ) then
-		current_dead_player.bones_inv:add_item("main", stk)
-	else
-		drop(current_dead_player.player_pos, stk)
-		current_dead_player.dropped=true
-	end
-end
-
-local function player_dies_transfer_inventory(player,transfer_stack)
 	local player_inv = player:get_inventory()
 	for _, list_name in ipairs(player_inventory_lists) do
 		for i = 1, player_inv:get_size(list_name) do
-			local stack = player_inv:get_stack(list_name, i)
+			table.insert(result, player_inv:get_stack(list_name, i))
-			transfer_stack(stack)
 		end
 		player_inv:set_list(list_name, {})
 	end
+	return result
 end
 
 bones.register_dead_player_inv_management(player_dies_transfer_inventory)
 
 minetest.register_on_dieplayer(function(player)
-	local pos = vector.round(player:get_pos())
+	local player_pos = vector.round(player:get_pos())
 	local bones_mode = minetest.settings:get("bones_mode") or "bones"
 	if bones_mode ~= "bones" and bones_mode ~= "drop" and bones_mode ~= "keep" then
 		bones_mode = "bones"
 	end
 	local player_name = player:get_player_name()
-	local current_dead_player={player=player, player_name=player_name, bones_inv=nil, bones_pos=nil,
+	local bones_inv = nil
-		bones_mode=bones_mode, player_pos=pos, dropped=false}
+	local bones_pos = nil
+	local dropped = false
+	local bones_meta = nil
 
 	local bones_position_message = minetest.settings:get_bool("bones_position_message") == true
-	local pos_string = minetest.pos_to_string(pos)
+	local pos_string = minetest.pos_to_string(player_pos)
 
 	-- return if keep inventory set or in creative mode
 	if bones_mode == "keep" or minetest.is_creative_enabled(player_name) then
@@ -279,73 +247,99 @@ minetest.register_on_dieplayer(function(player)
 		return
 	end
 
-	local callback=function(stk)
+	for _, fun in ipairs(dead_player_callbacks) do
-		transfer_stack_to_bones(stk,current_dead_player)
+		local items = fun(player)
+		for _, item in ipairs(items) do
+			-- check if it's possible to place bones, if not find space near player
+			if bones_mode == "bones" and bones_pos == nil then
+				bones_pos = player_pos
+				local air
+				if may_replace(bones_pos, player) then
+					air = bones_pos
+				else
+					air = minetest.find_node_near(bones_pos, 1, {"air"})
 				end
 
-	for i=1,#dead_player_callbacks do
+				if air and not minetest.is_protected(air, player_name) then
-		local fun=dead_player_callbacks[i]
+					bones_pos = air
-		fun(player,callback)
+					local param2 = minetest.dir_to_facedir(player:get_look_dir())
+					minetest.set_node(bones_pos, {name = "bones:bones", param2 = param2})
+					bones_meta = minetest.get_meta(bones_pos)
+					bones_inv = bones_meta:get_inventory()
+					--make it so big that anything reasonable will for sure fit inside
+					bones_inv:set_size("main", theoretical_max_slots)
+				else
+					bones_mode = "drop"
+					bones_pos = nil
+				end
+			end
+
+			if bones_mode == "bones" and bones_inv:room_for_item("main", item) then
+				bones_inv:add_item("main", item)
+			else
+				drop(player_pos, item)
+				dropped=true
+			end
+		end
 	end
 
 	local bones_conclusion
+	local public_conclusion
 
-	if not ( current_dead_player.bones_pos ) then
+	if not bones_pos then
-		drop(current_dead_player.player_pos, ItemStack("bones:bones"))
+		drop(player_pos, ItemStack("bones:bones"))
-		if not ( current_dead_player.dropped ) then
+		if not dropped then
 			bones_conclusion = "No bones placed"
-			if bones_position_message then
+			public_conclusion = S("@1 died at @2.", player_name, pos_string)
-				minetest.chat_send_player(player_name, S("@1 died at @2.", player_name, pos_string))
-			end
 		else
 			bones_conclusion = "Inventory dropped"
-			if bones_position_message then
+			public_conclusion = S("@1 died at @2, and dropped their inventory.", player_name, pos_string)
-				minetest.chat_send_player(player_name, S("@1 died at @2, and dropped their inventory.", player_name, pos_string, public_conclusion))
-			end
 		end
 	else
-		if not ( current_dead_player.dropped ) then
+		if not dropped then
 			bones_conclusion = "Bones placed"
-			if bones_position_message then
+			public_conclusion = S("@1 died at @2, and bones were placed.", player_name, pos_string)
-				minetest.chat_send_player(player_name, S("@1 died at @2, and bones were placed.", player_name, pos_string, public_conclusion))
-			end
 		else
 			bones_conclusion = "Inventory partially dropped"
+			public_conclusion = S("@1 died at @2, and partially dropped their inventory.", player_name, pos_string)
+		end
+	end
+
 	if bones_position_message then
-				minetest.chat_send_player(player_name, S("@1 died at @2, and partially dropped their inventory.", player_name, pos_string, public_conclusion))
+		minetest.chat_send_player(player_name, public_conclusion)
-			end
-		end
 	end
 
-	minetest.log("action", player_name .. " dies at " .. pos_string ..
+	minetest.log("action", player_name .. " dies at " .. pos_string .. ". " .. bones_conclusion)
-		". " .. bones_conclusion)
 
-	local inv = current_dead_player.bones_inv
+	if bones_inv then
 		local inv_size = theoretical_max_slots
-	if inv then
 		for i = 1, theoretical_max_slots do
-			local stack = inv:get_stack("main", i)
+			local stack = bones_inv:get_stack("main", i)
 			if stack:get_count() == 0 then
 				inv_size = i - 1
 				break
 			end
 		end
-		local meta = minetest.get_meta(current_dead_player.bones_pos)
+		if inv_size <= 4 * 8 then
-		meta:set_string("formspec", get_bones_formspec_for_size(inv_size))
+			bones_inv:set_size("main", 4 * 8)
-		meta:set_string("owner", player_name)
+		else
+			bones_inv:set_size("main", inv_size)
+		end
+		bones_meta:set_string("formspec", get_bones_formspec_for_size(inv_size))
+		bones_meta:set_string("owner", player_name)
 
 		if share_bones_time ~= 0 then
-			meta:set_string("infotext", S("@1's fresh bones", player_name))
+			bones_meta:set_string("infotext", S("@1's fresh bones", player_name))
 
-			if share_bones_time_early == 0 or not minetest.is_protected(pos, player_name) then
+			if share_bones_time_early == 0 or not minetest.is_protected(bones_pos, player_name) then
-				meta:set_int("time", 0)
+				bones_meta:set_int("time", 0)
 			else
-				meta:set_int("time", (share_bones_time - share_bones_time_early))
+				bones_meta:set_int("time", (share_bones_time - share_bones_time_early))
 			end
 
-			minetest.get_node_timer(pos):start(10)
+			minetest.get_node_timer(bones_pos):start(10)
 		else
-			meta:set_string("infotext", S("@1's bones", player_name))
+			bones_meta:set_string("infotext", S("@1's bones", player_name))
 		end
 	end
 end)