core: tidy up new safe_function implementation; bugfix

worldeditadditions_core/core/run_command.lua

@@ -2,10 +2,10 @@
 -- @module worldeditadditions_core
 
 -- WARNING: safe_region MUST NOT be imported more than once, as it defines chat commands. If you want to import it again elsewhere, check first that multiple dofile() calls don't execute a file more than once.
-local wea_c = worldeditadditions_core
-local safe_region = dofile(wea_c.modpath.."/core/safe_region.lua")
-local human_size = wea_c.format.human_size
-local safe_function = wea_c.safe_function
+local weac = worldeditadditions_core
+local safe_region = dofile(weac.modpath.."/core/safe_region.lua")
+local human_size = weac.format.human_size
+local safe_function = weac.safe_function
 
 -- TODO: Reimplement worldedit.player_notify(player_name, msg_text)
 
@@ -18,11 +18,17 @@ local safe_function = wea_c.safe_function
 -- @param	tbl_event	table		Internal event table used when calling `worldeditadditions_core.emit(event_name, tbl_event)`.
 -- @returns	nil
 local function run_command_stage2(player_name, func, parse_result, tbl_event)
-	wea_c:emit("pre-execute", tbl_event)
-	local success, result_message = func(player_name, wea_c.table.unpack(parse_result))
+	weac:emit("pre-execute", tbl_event)
+	local success_safefn, retvals = safe_function(func, { player_name, weac.table.unpack(parse_result) }, player_name, "The function crashed during execution.", tbl_event.cmdname)
+	if not success_safefn then return false end
+	
+	if #retvals ~= 2 then
+		worldedit.player_notify(player_name, "[//"..tostring(tbl_event.cmdname).."] The main execution function for this chat command returned "..tostring(#retvals).." arguments instead of the expected 2 (success, message), so it is unclear whether it succeeded or not. This is a bug!")
+	end
+	
+	local success, result_message = retvals[1], retvals[2]
 	print("DEBUG:run_command_stage2 SUCCESS", success, "RESULT_MESSAGE", result_message)
 	if not success then
-		
 		result_message = "[//"..tostring(tbl_event.cmdname).."] "..result_message
 	end
 	
@@ -32,7 +38,7 @@ local function run_command_stage2(player_name, func, parse_result, tbl_event)
 	end
 	tbl_event.success = success
 	tbl_event.result = result_message
-	wea_c:emit("post-execute", tbl_event)
+	weac:emit("post-execute", tbl_event)
 end
 
 
@@ -92,15 +98,15 @@ end
 -- @param	player_name	string	The name of the player to execute the command for.
 -- @param	paramtext	string	The unparsed argument string to pass to the command when executing it.
 local function run_command(cmdname, options, player_name, paramtext)
-	if options.require_pos > 0 and not worldedit.pos1[player_name] and not wea_c.pos.get1(player_name) then
+	if options.require_pos > 0 and not worldedit.pos1[player_name] and not weac.pos.get1(player_name) then
 		worldedit.player_notify(player_name, "Error: pos1 must be selected to use this command.")
 		return false
 	end
-	if options.require_pos > 1 and not worldedit.pos2[player_name] and not wea_c.pos.get2(player_name) then
+	if options.require_pos > 1 and not worldedit.pos2[player_name] and not weac.pos.get2(player_name) then
 		worldedit.player_notify(player_name, "Error: Both pos1 and pos2 must be selected (together making a region) to use this command.")
 		return false
 	end
-	local pos_count = wea_c.pos.count(player_name)
+	local pos_count = weac.pos.count(player_name)
 	if options.require_pos > 2 and pos_count < options.require_pos then
 		worldedit.player_notify(player_name, "Error: At least "..options.require_pos.."positions must be defined to use this command, but you only have "..pos_count.." defined (try using the multiwand).")
 		return false
@@ -113,37 +119,52 @@ local function run_command(cmdname, options, player_name, paramtext)
 		player_name = player_name
 	}
 	
-	wea_c:emit("pre-parse", tbl_event)
+	weac:emit("pre-parse", tbl_event)
+	
+	-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 	
 	-- local did_error = false
-	local success_safefn, success, parse_result = safe_function(options.parse, { paramtext }, player_name, "The command crashed when parsing the arguments.", cmdname)
+	local success_safefn, parse_result = safe_function(options.parse, { paramtext }, player_name, "The command crashed when parsing the arguments.", cmdname)
 	if not success_safefn then return false end -- error already sent to the player above
 	
-	print("DEBUG:run_command success_safefn", success_safefn, "success", success, "parse_result", parse_result)
-	
-	if not success then
-		worldedit.player_notify(player_name, ("[//"..tostring(cmdname).."] "..tostring(parse_result[1])) or "Invalid usage (no further error message was provided by the command. This is probably a bug.)")
+	if #parse_result == 0 then 
+		worldedit.player_notify(player_name, "[//"..tostring(cmdname).."] No return values at all were returned by the parsing function - not even a success boolean. This is a bug - please report it :D")
 		return false
 	end
 	
+	local success = table.remove(parse_result, 1)
+	if not success then
+		worldedit.player_notify(player_name, "[//"..tostring(cmdname).."] "..(tostring(parse_result[1]) or "Invalid usage (no further error message was provided by the command. This is probably a bug.)"))
+		return false
+	end
+	
+	-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~	
+	
 	tbl_event.paramargs = parse_result
-	wea_c:emit("post-parse", tbl_event)
+	weac:emit("post-parse", tbl_event)
 	
 	
 	if options.nodes_needed then
-		local potential_changes = options.nodes_needed(player_name, wea_c.table.unpack(parse_result))
+		local success_xpcall_nn, retvals_nn = safe_function(options.nodes_needed, { player_name, weac.table.unpack(parse_result) }, player_name, "The nodes_needed function crashed!", cmdname)
+		if not success_xpcall_nn then return false end
+		
+		if #retvals_nn == 0 then
+			worldedit.player_notify(player_name, "[//"..tostring(cmdname).."] Error: The nodes_needed function didn't return any values. This is a bug!")
+			return false
+		end
+		local potential_changes = retvals_nn[1]
 		
 		tbl_event.potential_changes = potential_changes
-		wea_c:emit("post-nodesneeded", tbl_event)
+		weac:emit("post-nodesneeded", tbl_event)
 		
 		if type(potential_changes) ~= "number" then
 			worldedit.player_notify(player_name, "Error: The command '"..cmdname.."' returned a "..type(potential_changes).." instead of a number when asked how many nodes might be changed. Abort. This is a bug.")
 			return
 		end
 		
-		local limit = wea_c.safe_region_limit_default
-		if wea_c.safe_region_limits[player_name] then
-			limit = wea_c.safe_region_limits[player_name]
+		local limit = weac.safe_region_limit_default
+		if weac.safe_region_limits[player_name] then
+			limit = weac.safe_region_limits[player_name]
 		end
 		if type(potential_changes) == "string" then
 			worldedit.player_notify(player_name, "/"..cmdname.." "..paramtext.." "..potential_changes..". Type //y to continue, or //n to cancel (in this specific situation, your configured limit via the //saferegion command does not apply).")

worldeditadditions_core/core/safe_function.lua

@@ -69,7 +69,7 @@ end
 -- @param	string|nil	player_name		The name of the player affected. If nil then no message is sent to the player.
 -- @param	string		error_msg		The error message to send when `fn` inevitably crashes.
 -- @param	string|nil		cmdname		Optional. The name of the command being run.
--- @returns	bool,any,...	A success bool (true == success), and then if success == true the rest of the arguments are the (unpacked) return values from the function called.
+-- @returns	bool,any,...	A success bool (true == success), and then if success == true the rest of the arguments are the (unpacked) return values from the function called. If success == false, then the 2nd argument will be the stack trace.
 function safe_function(fn, args, player_name, error_msg, cmdname)
 	local retvals
 	local success_xpcall, stack_trace = xpcall(function()
@@ -78,10 +78,20 @@ function safe_function(fn, args, player_name, error_msg, cmdname)
 	
 	if not success_xpcall then
 		send_error(player_name, cmdname, error_msg, stack_trace)
-		return false
+		weac:emit("error", {
+			fn = fn,
+			args = args,
+			player_name = player_name,
+			cmdname = cmdname,
+			stack_trace = stack_trace,
+			error_msg = error_msg
+		})
+		return false, stack_trace
 	end
 	
-	return true, weac.table.unpack(retvals)
+	minetest.log("error", "[//"..tostring(cmdname).."] Caught error from running function ", fn, "with args", weac.inspect(args), "for player ", player_name, "with provided error message", error_msg, ". Stack trace: ", stack_trace)
+	
+	return true, retvals
 end
 
 

worldeditadditions_core/utils/format/escape.lua

@@ -10,7 +10,6 @@
 -- TODO this doesn't work. It replaces \n with %A instead of %0A, though we don't know if that's a problem or not
 -- it also doesn't handle quotes even though we've clearly got them in the Lua pattern
 local function _escape_char(char)
-	print("_escape_char char", char, "result", string.format('%%%02X', string.byte(char)))
 	return string.format('%%%02X', string.byte(char))
 end