new code for lua protected calls in lua_kernel_base

In initial commit of the new lua kernels, I introduced a problem
by trying to use luaW_pcall and lua_pcall interchangeably via
polymorphism. This doesn't work because their return types don't
match, and it's alot of work to change luaW_pcall syntax. Besides
this there's no reason we can't use the custom error handler
everywhere.

This commits adds protected_call and load_string functions to lua
kernel base. These are an intended replacement for luaW_pcall,
and replace the "run" function. They do better error reporting
and allow to specify an error handler.

The error reporting is very flexible -- by default we select a
an error reporting function associated polymorphically to the lua
kernel, so the in-game lua kernel can send chat messages, and
others can do something else. However an arbitrary handler may
be specified, and exceptions instead of logging may also be
requested.

src/game_errors.hpp

@@ -47,6 +47,14 @@ struct game_error : public error {
 	game_error(const std::string& msg) : error("game_error: " + msg) {}
 };
 
+/**
+ * Error used to report an error in a lua script or in the lua interpreter.
+ */
+struct lua_error : public error {
+	lua_error(const std::string& msg) : error("lua_error: " + msg) {}
+	lua_error(const std::string& msg, const std::string& context) : error(context + ": " + msg) {}
+};
+
 /**
  * Exception used to signal that the user has decided to abort a game,
  * and to load another game instead.

src/scripting/application_lua_kernel.cpp

@@ -104,8 +104,7 @@ void application_lua_kernel::call_script(const config & event_cfg) {
 
 	luaW_pushconfig(L, event_cfg); //push the config as an argument
 
-	pcall_fcn_ptr pcall = pcall_fcn();
-	if (!pcall(L, 1, 0)) //call the script from protected mode, there is one argument and we expect no return values.
+	if (!luaW_pcall(L, 1, 0)) //call the script from protected mode, there is one argument and we expect no return values.
 	{
 		WRN_LUA << "Got an error when executing script:\n" << lua_tostring(L,-1) << std::endl;
 	}

src/scripting/game_lua_kernel.cpp

@@ -153,6 +153,13 @@ void extract_preload_scripts(config const &game_config)
 	preload_config = game_config.child("game_config");
 }
 
+void LuaKernel::log_error(char const * msg, char const * context)
+{
+	lua_kernel_base::log_error(msg, context);
+	chat_message(context, msg);
+}
+
+
 namespace {
 	/**
 	 * Stack storing the queued_event objects needed for calling WML actions.
@@ -3717,14 +3724,6 @@ LuaKernel::LuaKernel(const config &cfg)
 	lua_setfield(L, -2, "theme_items");
 	lua_pop(L, 1);
 
-	// Store the error handler.
-	lua_pushlightuserdata(L
-			, executeKey);
-	lua_getglobal(L, "debug");
-	lua_getfield(L, -1, "traceback");
-	lua_remove(L, -2);
-	lua_rawset(L, LUA_REGISTRYINDEX);
-
 	lua_settop(L, 0);
 }
 
@@ -3795,10 +3794,10 @@ void LuaKernel::initialize()
 	// Execute the preload scripts.
 	game_config::load_config(preload_config);
 	BOOST_FOREACH(const config &cfg, preload_scripts) {
-		execute(cfg["code"].str().c_str(), 0, 0);
+		run(cfg["code"].str().c_str());
 	}
 	BOOST_FOREACH(const config &cfg, level_.child_range("lua")) {
-		execute(cfg["code"].str().c_str(), 0, 0);
+		run(cfg["code"].str().c_str());
 	}
 
 	load_game();
@@ -3886,8 +3885,7 @@ void LuaKernel::save_game(config &cfg)
 			 * and the extra UMC ones.
 			 */
 			const std::string m = "Tag is already used: [" + i->key + "]";
-			chat_message("Lua error", m);
-			ERR_LUA << m << '\n';
+			log_error(m.c_str());
 			v.erase(i);
 			continue;
 		}
@@ -3979,8 +3977,7 @@ bool LuaKernel::run_filter(char const *name, unit const &u)
 	if(!luaW_getglobal(L, name, NULL))
 	{
 		std::string message = std::string() + "function " + name + " not found";
-		chat_message("Lua SUF Error", message);
-		ERR_LUA << "Lua SUF Error: " << message << std::endl;
+		log_error(message.c_str(), "Lua SUF Error");
 		//we pushed nothing and can safeley return.
 		return false;
 	}
@@ -3998,13 +3995,6 @@ bool LuaKernel::run_filter(char const *name, unit const &u)
 	return b;
 }
 
-// This is needed because default args don't work through function pointers
-static int luaW_pcall_default_args(lua_State * L, int a, int b) {
-	return luaW_pcall(L,a,b,false);
-}
-
-pcall_fcn_ptr LuaKernel::pcall_fcn() { return &luaW_pcall_default_args; } //this causes the run() function to use luaW_pcall instead of lua_pcall
-
 ai::lua_ai_context* LuaKernel::create_lua_ai_context(char const *code, ai::engine_lua *engine)
 {
 	return ai::lua_ai_context::create(mState,code,engine);

src/scripting/game_lua_kernel.hpp

@@ -47,7 +47,7 @@ public:
 		game_events::queued_event const &);
 	bool run_filter(char const *name, unit const &u);
 
-	virtual pcall_fcn_ptr pcall_fcn();
+	virtual void log_error(char const* msg, char const* context = "Lua error");
 
 	ai::lua_ai_context* create_lua_ai_context(char const *code, ai::engine_lua *engine);
 	ai::lua_ai_action_handler* create_lua_ai_action_handler(char const *code, ai::lua_ai_context &context);

src/scripting/lua_kernel_base.cpp

@@ -20,6 +20,7 @@
 #include "lua/lauxlib.h"
 #include "lua/lua.h"
 #include "lua/lualib.h"
+#include "game_errors.hpp"
 
 #ifdef DEBUG_LUA
 #include "scripting/debug_lua.hpp"
@@ -29,6 +30,7 @@
 
 #include <cstring>
 #include <string>
+#include <boost/bind.hpp>
 
 static lg::log_domain log_scripting_lua("scripting/lua");
 #define LOG_LUA LOG_STREAM(info, log_scripting_lua)
@@ -82,6 +84,14 @@ lua_kernel_base::lua_kernel_base()
 	}
 	lua_pop(L, 1);
 
+	// Store the error handler.
+	lua_pushlightuserdata(L
+			, executeKey);
+	lua_getglobal(L, "debug");
+	lua_getfield(L, -1, "traceback");
+	lua_remove(L, -2);
+	lua_rawset(L, LUA_REGISTRYINDEX);
+
 	lua_settop(L, 0);
 }
 
@@ -90,51 +100,105 @@ lua_kernel_base::~lua_kernel_base()
 	lua_close(mState);
 }
 
-// This is needed because lua_pcall is actually a macro
-static int lua_pcall_fcn(lua_State * L, int a, int b)
+void lua_kernel_base::log_error(char const * msg, char const * context)
 {
-	return lua_pcall(L,a,b,0);
+	ERR_LUA << context << ": " << msg;
 }
 
-// In the "base" configuration we just want to call lua_pcall when running scripts.
-pcall_fcn_ptr lua_kernel_base::pcall_fcn() { return &lua_pcall_fcn; }
+void lua_kernel_base::throw_exception(char const * msg, char const * context)
+{
+	throw game::lua_error(msg, context);
+}
 
-/**
- * Runs a script on a stack containing @a nArgs arguments.
- * @return true if the script was successful and @a nRets return values are available.
- */
-bool lua_kernel_base::execute(char const *prog, int nArgs, int nRets)
+bool lua_kernel_base::protected_call(int nArgs, int nRets)
+{
+	error_handler eh = boost::bind(&lua_kernel_base::log_error, this, _1, _2 );
+	return protected_call(nArgs, nRets, eh);
+}
+
+bool lua_kernel_base::load_string(char const * prog)
+{
+	error_handler eh = boost::bind(&lua_kernel_base::log_error, this, _1, _2 );
+	return load_string(prog, eh);
+}
+
+bool lua_kernel_base::protected_call(int nArgs, int nRets, error_handler e_h)
 {
 	lua_State *L = mState;
 
-	// Compile script into a variadic function.
-	int res = luaL_loadstring(L, prog);
-	if (res)
-	{
-		char const *m = lua_tostring(L, -1);
-		chat_message("Lua error", m);
-		ERR_LUA << m << '\n';
-		lua_pop(L, 1);
-		return true;
+	// Load the error handler before the function and its arguments.
+	lua_pushlightuserdata(L
+			, executeKey);
+
+	lua_rawget(L, LUA_REGISTRYINDEX);
+	lua_insert(L, -2 - nArgs);
+
+	int error_handler_index = lua_gettop(L) - nArgs - 1;
+
+	// Call the function.
+	int errcode = lua_pcall(L, nArgs, nRets, -2 - nArgs);
+	tlua_jailbreak_exception::rethrow();
+
+	// Remove the error handler.
+	lua_remove(L, error_handler_index);
+
+	if (errcode != LUA_OK) {
+		std::string message = lua_tostring(L, -1);
+
+		std::string context = "When executing, ";
+		if (errcode == LUA_ERRRUN) {
+			context += "Lua runtime error: ";
+		} else if (errcode == LUA_ERRERR) {
+			context += "Lua error in attached debugger: ";
+		} else if (errcode == LUA_ERRMEM) {
+			context += "Lua out of memory error: ";
+		} else if (errcode == LUA_ERRGCMM) {
+			context += "Lua error in garbage collection metamethod: ";
+		} else {
+			context += "unknown lua error: ";
+		}
+
+		e_h(message.c_str(), context.c_str());
+
+		return false;
 	}
 
-	// Place the function before its arguments.
-	if (nArgs)
-		lua_insert(L, -1 - nArgs);
-
-	pcall_fcn_ptr f = pcall_fcn();
-	return f(L, nArgs, nRets);
+	return true;
 }
 
+bool lua_kernel_base::load_string(char const * prog, error_handler e_h)
+{
+	int errcode = luaL_loadstring(mState, prog);
+	if (errcode != LUA_OK) {
+		std::string msg = lua_tostring(mState, -1);
+
+		std::string context = "When parsing a string to lua, ";
+
+		if (errcode == LUA_ERRSYNTAX) {
+			msg += " a syntax error: ";
+		} else if(errcode == LUA_ERRMEM){
+			msg += " a memory error: ";
+		} else if(errcode == LUA_ERRGCMM) {
+			msg += " an error in garbage collection metamethod: ";
+		} else {
+			msg += " an unknown error: ";
+		}
+
+		e_h(msg.c_str(), context.c_str());
+
+		return false;
+	}
+	return true;
+}
+
+// Call load_string and protected call. Make them throw exceptions, and if we catch one, reformat it with signature for this function and log it.
 void lua_kernel_base::run(const char * prog) {
-	if (execute(prog, 0, 0)) {
-		lua_State *L = mState;
-
-		char const *m = lua_tostring(L, -1);
-		ERR_LUA << "lua_kernel::run(): " << m << '\n';
-		lua_pop(L,1);
-
-		//execute("print(debug.traceback())",0,0);
+	try {
+		error_handler eh = boost::bind(&lua_kernel_base::throw_exception, this, _1, _2 );
+		load_string(prog, eh);
+		protected_call(0, 0, eh);
+	} catch (game::lua_error & e) {
+		lua_kernel_base::log_error(e.what(), "In function lua_kernel::run()");
 	}
 }
 

src/scripting/lua_kernel_base.hpp

@@ -16,25 +16,35 @@
 #define SCRIPTING_LUA_KERNEL_BASE_HPP
 
 #include <string>                       // for string
+#include "utils/boost_function_guarded.hpp"
 
 struct lua_State;
 
-typedef int (*pcall_fcn_ptr)(lua_State *, int, int);
-
 class lua_kernel_base {
-protected:
-	lua_State *mState;
-	bool execute(char const *, int, int);
 public:
 	lua_kernel_base();
 	virtual ~lua_kernel_base();
 
-	/** Runs a plain script. */
+	/** Runs a plain script. Doesn't throw lua_error.*/
 	void run(char const *prog);
 
 	void load_package();
 
-	virtual pcall_fcn_ptr pcall_fcn(); //when running scripts, in the "base" kernel type we should just use pcall. But for the in-game kernel, we want to call the luaW_pcall function instead which extends it using things specific to that api, and returns errors on a WML channel
+	virtual void log_error(char const* msg, char const* context = "Lua error");
+	virtual void throw_exception(char const* msg, char const* context = "Lua error"); //throws game::lua_error
+
+	typedef boost::function<void(char const*, char const*)> error_handler;
+
+protected:
+	lua_State *mState;
+
+	// Execute a protected call. Error handler is called in case of an error, using syntax for log_error and throw_exception above. Returns true if successful.
+	bool protected_call(int nArgs, int nRets, error_handler);
+	// Load a string onto the stack as a function. Returns true if successful, error handler is called if not.
+	bool load_string(char const * prog, error_handler);
+
+	virtual bool protected_call(int nArgs, int nRets); 	// select default error handler polymorphically
+	virtual bool load_string(char const * prog);		// select default error handler polymorphically
 };
 
 #endif