Merge pull request #532 from mc-server/LuaStateErrorHandler

Lua state error handler

VC2008/MCServer.vcproj

@@ -2025,10 +2025,6 @@
 					RelativePath="..\src\Bindings\PluginManager.h"
 					>
 				</File>
-				<File
-					RelativePath="..\src\Bindings\tolua_base.h"
-					>
-				</File>
 				<File
 					RelativePath="..\src\Bindings\WebPlugin.cpp"
 					>

src/Bindings/AllToLua.pkg

@@ -1,8 +1,6 @@
 
 $#include "../Globals.h"
 
-$#include "tolua_base.h"
-
 // Typedefs from Globals.h, so that we don't have to process that file:
 typedef long long Int64;
 typedef int       Int32;

src/Bindings/LuaState.cpp

@@ -228,6 +228,9 @@ bool cLuaState::PushFunction(const char * a_FunctionName)
 		return false;
 	}
 	
+	// Push the error handler for lua_pcall()
+	lua_pushcfunction(m_LuaState, &ReportFnCallErrors);
+	
 	lua_getglobal(m_LuaState, a_FunctionName);
 	if (!lua_isfunction(m_LuaState, -1))
 	{
@@ -249,6 +252,9 @@ bool cLuaState::PushFunction(int a_FnRef)
 	ASSERT(IsValid());
 	ASSERT(m_NumCurrentFunctionArgs == -1);  // If not, there's already something pushed onto the stack
 	
+	// Push the error handler for lua_pcall()
+	lua_pushcfunction(m_LuaState, &ReportFnCallErrors);
+	
 	lua_rawgeti(m_LuaState, LUA_REGISTRYINDEX, a_FnRef);  // same as lua_getref()
 	if (!lua_isfunction(m_LuaState, -1))
 	{
@@ -264,27 +270,29 @@ bool cLuaState::PushFunction(int a_FnRef)
 
 
 
-bool cLuaState::PushFunctionFromRefTable(cRef & a_TableRef, const char * a_FnName)
+bool cLuaState::PushFunction(const cTableRef & a_TableRef)
 {
 	ASSERT(IsValid());
 	ASSERT(m_NumCurrentFunctionArgs == -1);  // If not, there's already something pushed onto the stack
 	
-	lua_rawgeti(m_LuaState, LUA_REGISTRYINDEX, a_TableRef);  // Get the table ref
+	// Push the error handler for lua_pcall()
+	lua_pushcfunction(m_LuaState, &ReportFnCallErrors);
+	
+	lua_rawgeti(m_LuaState, LUA_REGISTRYINDEX, a_TableRef.GetTableRef());  // Get the table ref
 	if (!lua_istable(m_LuaState, -1))
 	{
 		// Not a table, bail out
 		lua_pop(m_LuaState, 1);
 		return false;
 	}
-	lua_getfield(m_LuaState, -1, a_FnName);
+	lua_getfield(m_LuaState, -1, a_TableRef.GetFnName());
 	if (lua_isnil(m_LuaState, -1) || !lua_isfunction(m_LuaState, -1))
 	{
 		// Not a valid function, bail out
 		lua_pop(m_LuaState, 2);
 		return false;
 	}
-	lua_remove(m_LuaState, -2);  // Remove the table ref from the stack
+	Printf(m_CurrentFunctionName, "<table-callback %s>", a_TableRef.GetFnName());
-	m_CurrentFunctionName = "<table_callback>";
 	m_NumCurrentFunctionArgs = 0;
 	return true;
 }
@@ -732,11 +740,13 @@ void cLuaState::GetReturn(int a_StackPos, double & a_ReturnedVal)
 bool cLuaState::CallFunction(int a_NumResults)
 {
 	ASSERT (m_NumCurrentFunctionArgs >= 0);  // A function must be pushed to stack first
-	ASSERT(lua_isfunction(m_LuaState, -m_NumCurrentFunctionArgs - 1));
+	ASSERT(lua_isfunction(m_LuaState, -m_NumCurrentFunctionArgs - 1));  // The function to call
+	ASSERT(lua_isfunction(m_LuaState, -m_NumCurrentFunctionArgs - 2));  // The error handler
 	
-	int s = lua_pcall(m_LuaState, m_NumCurrentFunctionArgs, a_NumResults, 0);
+	int s = lua_pcall(m_LuaState, m_NumCurrentFunctionArgs, a_NumResults, -m_NumCurrentFunctionArgs - 2);
-	if (ReportErrors(s))
+	if (s != 0)
 	{
+		// The error has already been printed together with the stacktrace
 		LOGWARNING("Error in %s calling function %s()", m_SubsystemName.c_str(), m_CurrentFunctionName.c_str());
 		m_NumCurrentFunctionArgs = -1;
 		m_CurrentFunctionName.clear();
@@ -963,16 +973,25 @@ bool cLuaState::ReportErrors(lua_State * a_LuaState, int a_Status)
 
 
 void cLuaState::LogStackTrace(void)
+{
+	LogStackTrace(m_LuaState);
+}
+
+
+
+
+
+void cLuaState::LogStackTrace(lua_State * a_LuaState)
 {
 	LOGWARNING("Stack trace:");
 	lua_Debug entry;
 	int depth = 0;
-	while (lua_getstack(m_LuaState, depth, &entry))
+	while (lua_getstack(a_LuaState, depth, &entry))
 	{
-		int status = lua_getinfo(m_LuaState, "Sln", &entry);
+		int status = lua_getinfo(a_LuaState, "Sln", &entry);
 		assert(status);
 
-		LOGWARNING("  %s(%d): %s", entry.short_src, entry.currentline, entry.name ? entry.name : "?");
+		LOGWARNING("  %s(%d): %s", entry.short_src, entry.currentline, entry.name ? entry.name : "(no name)");
 		depth++;
 	}
 	LOGWARNING("Stack trace end");
@@ -1005,6 +1024,17 @@ AString cLuaState::GetTypeText(int a_StackPos)
 
 
 
+int cLuaState::ReportFnCallErrors(lua_State * a_LuaState)
+{
+	LOGWARNING("LUA: %s", lua_tostring(a_LuaState, -1));
+	LogStackTrace(a_LuaState);
+	return 1;  // We left the error message on the stack as the return value
+}
+
+
+
+
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // cLuaState::cRef:
 

src/Bindings/LuaState.h

@@ -85,6 +85,23 @@ public:
 	} ;
 	
 	
+	/** Used for calling functions stored in a reference-stored table */
+	class cTableRef
+	{
+		int m_TableRef;
+		const char * m_FnName;
+	public:
+		cTableRef(int a_TableRef, const char * a_FnName) :
+			m_TableRef(a_TableRef),
+			m_FnName(a_FnName)
+		{
+		}
+		
+		int GetTableRef(void) const { return m_TableRef; }
+		const char * GetFnName(void) const { return m_FnName; }
+	} ;
+	
+	
 	/// A dummy class that's used only to delimit function args from return values for cLuaState::Call()
 	class cRet
 	{
@@ -133,24 +150,6 @@ public:
 	/// Returns true if a_FunctionName is a valid Lua function that can be called
 	bool HasFunction(const char * a_FunctionName);
 	
-	/** Pushes the function of the specified name onto the stack.
-	Returns true if successful. Logs a warning on failure (incl. m_SubsystemName)
-	*/
-	bool PushFunction(const char * a_FunctionName);
-	
-	/** Pushes a function that has been saved into the global registry, identified by a_FnRef.
-	Returns true if successful. Logs a warning on failure
-	*/
-	bool PushFunction(int a_FnRef);
-	
-	/** Pushes a function that is stored in a table ref.
-	Returns true if successful, false on failure. Doesn't log failure.
-	*/
-	bool PushFunctionFromRefTable(cRef & a_TableRef, const char * a_FnName);
-	
-	/// Pushes a usertype of the specified class type onto the stack
-	void PushUserType(void * a_Object, const char * a_Type);
-
 	// Push a value onto the stack
 	void Push(const AString & a_String);
 	void Push(const AStringVector & a_Vector);
@@ -802,25 +801,6 @@ public:
 	}
 
 
-	/// Retrieve value returned at a_StackPos, if it is a valid bool. If not, a_ReturnedVal is unchanged
-	void GetReturn(int a_StackPos, bool & a_ReturnedVal);
-	
-	/// Retrieve value returned at a_StackPos, if it is a valid string. If not, a_ReturnedVal is unchanged
-	void GetReturn(int a_StackPos, AString & a_ReturnedVal);
-	
-	/// Retrieve value returned at a_StackPos, if it is a valid number. If not, a_ReturnedVal is unchanged
-	void GetReturn(int a_StackPos, int & a_ReturnedVal);
-	
-	/// Retrieve value returned at a_StackPos, if it is a valid number. If not, a_ReturnedVal is unchanged
-	void GetReturn(int a_StackPos, double & a_ReturnedVal);
-	
-	/**
-	Calls the function that has been pushed onto the stack by PushFunction(),
-	with arguments pushed by PushXXX().
-	Returns true if successful, logs a warning on failure.
-	*/
-	bool CallFunction(int a_NumReturnValues);
-	
 	/// Returns true if the specified parameters on the stack are of the specified usertable type; also logs warning if not. Used for static functions
 	bool CheckParamUserTable(int a_StartParam, const char * a_UserTable, int a_EndParam = -1);
 	
@@ -848,6 +828,9 @@ public:
 	/// Logs all items in the current stack trace to the server console
 	void LogStackTrace(void);
 	
+	/// Logs all items in the current stack trace to the server console
+	static void LogStackTrace(lua_State * a_LuaState);
+	
 	/// Returns the type of the item on the specified position in the stack
 	AString GetTypeText(int a_StackPos);
 	
@@ -867,6 +850,47 @@ protected:
 	
 	/// Number of arguments currently pushed (for the Push / Call chain)
 	int m_NumCurrentFunctionArgs;
+
+
+	/** Pushes the function of the specified name onto the stack.
+	Returns true if successful. Logs a warning on failure (incl. m_SubsystemName)
+	*/
+	bool PushFunction(const char * a_FunctionName);
+	
+	/** Pushes a function that has been saved into the global registry, identified by a_FnRef.
+	Returns true if successful. Logs a warning on failure
+	*/
+	bool PushFunction(int a_FnRef);
+	
+	/** Pushes a function that is stored in a referenced table by name
+	Returns true if successful. Logs a warning on failure
+	*/
+	bool PushFunction(const cTableRef & a_TableRef);
+	
+	/// Pushes a usertype of the specified class type onto the stack
+	void PushUserType(void * a_Object, const char * a_Type);
+
+	/// Retrieve value returned at a_StackPos, if it is a valid bool. If not, a_ReturnedVal is unchanged
+	void GetReturn(int a_StackPos, bool & a_ReturnedVal);
+	
+	/// Retrieve value returned at a_StackPos, if it is a valid string. If not, a_ReturnedVal is unchanged
+	void GetReturn(int a_StackPos, AString & a_ReturnedVal);
+	
+	/// Retrieve value returned at a_StackPos, if it is a valid number. If not, a_ReturnedVal is unchanged
+	void GetReturn(int a_StackPos, int & a_ReturnedVal);
+	
+	/// Retrieve value returned at a_StackPos, if it is a valid number. If not, a_ReturnedVal is unchanged
+	void GetReturn(int a_StackPos, double & a_ReturnedVal);
+	
+	/**
+	Calls the function that has been pushed onto the stack by PushFunction(),
+	with arguments pushed by PushXXX().
+	Returns true if successful, logs a warning on failure.
+	*/
+	bool CallFunction(int a_NumReturnValues);
+	
+	/** Used as the error reporting function for function calls */
+	static int ReportFnCallErrors(lua_State * a_LuaState);
 } ;
 
 

src/Bindings/ManualBindings.cpp

@@ -1980,118 +1980,72 @@ public:
 	
 	virtual bool OnNextBlock(int a_BlockX, int a_BlockY, int a_BlockZ, BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, char a_EntryFace) override
 	{
-		if (!m_LuaState.PushFunctionFromRefTable(m_TableRef, "OnNextBlock"))
+		bool res = false;
+		if (!m_LuaState.Call(
+			cLuaState::cTableRef(m_TableRef, "OnNextBlock"),
+			a_BlockX, a_BlockY, a_BlockZ, a_BlockType, a_BlockMeta, a_EntryFace,
+			cLuaState::Return, res
+		))
 		{
 			// No such function in the table, skip the callback
 			return false;
 		}
-		m_LuaState.Push(a_BlockX);
-		m_LuaState.Push(a_BlockY);
-		m_LuaState.Push(a_BlockZ);
-		m_LuaState.Push(a_BlockType);
-		m_LuaState.Push(a_BlockMeta);
-		m_LuaState.Push(a_EntryFace);
-		if (!m_LuaState.CallFunction(1))
-		{
-			return false;
-		}
-		bool res = false;
-		if (lua_isboolean(m_LuaState, -1))
-		{
-			res = (lua_toboolean(m_LuaState, -1) != 0);
-		}
-		lua_pop(m_LuaState, 1);
 		return res;
 	}
 	
 	virtual bool OnNextBlockNoData(int a_BlockX, int a_BlockY, int a_BlockZ, char a_EntryFace) override
 	{
-		if (!m_LuaState.PushFunctionFromRefTable(m_TableRef, "OnNextBlockNoData"))
+		bool res = false;
+		if (!m_LuaState.Call(
+			cLuaState::cTableRef(m_TableRef, "OnNextBlockNoData"),
+			a_BlockX, a_BlockY, a_BlockZ, a_EntryFace,
+			cLuaState::Return, res
+		))
 		{
 			// No such function in the table, skip the callback
 			return false;
 		}
-		m_LuaState.Push(a_BlockX);
-		m_LuaState.Push(a_BlockY);
-		m_LuaState.Push(a_BlockZ);
-		m_LuaState.Push(a_EntryFace);
-		if (!m_LuaState.CallFunction(1))
-		{
-			return false;
-		}
-		bool res = false;
-		if (lua_isboolean(m_LuaState, -1))
-		{
-			res = (lua_toboolean(m_LuaState, -1) != 0);
-		}
-		lua_pop(m_LuaState, 1);
 		return res;
 	}
 	
 	virtual bool OnOutOfWorld(double a_BlockX, double a_BlockY, double a_BlockZ) override
 	{
-		if (!m_LuaState.PushFunctionFromRefTable(m_TableRef, "OnOutOfWorld"))
+		bool res = false;
+		if (!m_LuaState.Call(
+			cLuaState::cTableRef(m_TableRef, "OnOutOfWorld"),
+			a_BlockX, a_BlockY, a_BlockZ,
+			cLuaState::Return, res
+		))
 		{
 			// No such function in the table, skip the callback
 			return false;
 		}
-		m_LuaState.Push(a_BlockX);
-		m_LuaState.Push(a_BlockY);
-		m_LuaState.Push(a_BlockZ);
-		if (!m_LuaState.CallFunction(1))
-		{
-			return false;
-		}
-		bool res = false;
-		if (lua_isboolean(m_LuaState, -1))
-		{
-			res = (lua_toboolean(m_LuaState, -1) != 0);
-		}
-		lua_pop(m_LuaState, 1);
 		return res;
 	}
 	
 	virtual bool OnIntoWorld(double a_BlockX, double a_BlockY, double a_BlockZ) override
 	{
-		if (!m_LuaState.PushFunctionFromRefTable(m_TableRef, "OnIntoWorld"))
+		bool res = false;
+		if (!m_LuaState.Call(
+			cLuaState::cTableRef(m_TableRef, "OnIntoWorld"),
+			a_BlockX, a_BlockY, a_BlockZ,
+			cLuaState::Return, res
+		))
 		{
 			// No such function in the table, skip the callback
 			return false;
 		}
-		m_LuaState.Push(a_BlockX);
-		m_LuaState.Push(a_BlockY);
-		m_LuaState.Push(a_BlockZ);
-		if (!m_LuaState.CallFunction(1))
-		{
-			return false;
-		}
-		bool res = false;
-		if (lua_isboolean(m_LuaState, -1))
-		{
-			res = (lua_toboolean(m_LuaState, -1) != 0);
-		}
-		lua_pop(m_LuaState, 1);
 		return res;
 	}
 	
 	virtual void OnNoMoreHits(void) override
 	{
-		if (!m_LuaState.PushFunctionFromRefTable(m_TableRef, "OnNoMoreHits"))
+		m_LuaState.Call(cLuaState::cTableRef(m_TableRef, "OnNoMoreHits"));
-		{
-			// No such function in the table, skip the callback
-			return;
-		}
-		m_LuaState.CallFunction(0);
 	}
 	
 	virtual void OnNoChunk(void) override
 	{
-		if (!m_LuaState.PushFunctionFromRefTable(m_TableRef, "OnNoChunk"))
+		m_LuaState.Call(cLuaState::cTableRef(m_TableRef, "OnNoChunk"));
-		{
-			// No such function in the table, skip the callback
-			return;
-		}
-		m_LuaState.CallFunction(0);
 	}
 
 protected:

src/Bindings/tolua_base.h

@@ -1,128 +0,0 @@
-#ifndef TOLUA_BASE_H
-#define TOLUA_BASE_H
-
-#pragma warning(disable:4800) // This file is ONLY included by Bindings.cpp and it throws lots of C4800 warnings
-
-#include "tolua++/include/tolua++.h"
-
-
-
-
-
-class ToluaBase {
-
-	int lua_instance;
-
-protected:
-
-	lua_State* lua_state;
-
-	void lua_stacktrace(lua_State* L) const
-	{
-		lua_Debug entry;
-		int depth = 0;
-
-		while (lua_getstack(L, depth, &entry))
-		{
-			lua_getinfo(L, "Sln", &entry);
-
-			LOGERROR("%s(%d): %s", entry.short_src, entry.currentline, entry.name ? entry.name : "?");
-			depth++;
-		}
-	}
-
-
-	bool report_errors(int status) const
-	{
-		if ( status!=0 )
-		{
-			const char* s = lua_tostring(lua_state, -1);
-			LOGERROR("-- %s", s );
-			//lua_pop(lua_state, 1);
-			LOGERROR("Stack:");
-			lua_stacktrace( lua_state );
-			return true;
-		}
-		return false;
-	}
-
-	bool push_method(const char* name, lua_CFunction f) const {
-
-		if (!lua_state) return false;
-
-		lua_getref(lua_state, lua_instance);
-		lua_pushstring(lua_state, name);
-		//LOGINFO("1. push_method() Stack size: %i", lua_gettop( lua_state ) );
-		lua_gettable(lua_state, -2);
-		//LOGINFO("2. push_method() Stack size: %i", lua_gettop( lua_state ) );
-
-		if (lua_isnil(lua_state, -1)) {
-
-			// pop the table
-			lua_pop(lua_state, 2);
-			return false;
-
-		} else {
-
-			if (f) {
-				if (lua_iscfunction(lua_state, -1)) {
-					lua_pop(lua_state, 2);
-					return false;
-				};
-				/* // not for now
-				lua_pushcfunction(lua_state, f);
-				if (lua_rawequal(lua_state, -1, -2)) {
-
-					// avoid recursion, pop both functions and the table
-					lua_pop(lua_state, 3);
-					return false;
-				};
-
-				// pop f
-				lua_pop(lua_state, 1);
-				*/
-			};
-
-			// swap table with function
-			lua_insert(lua_state, -2);
-		};
-
-		return true;
-	};
-
-	void dbcall(lua_State* L, int nargs, int nresults) const {
-
-		// using lua_call for now
-		int s = lua_pcall(L, nargs, nresults, 0);
-		report_errors( s );
-	};
-public:
-	
-	int GetInstance() { return lua_instance; }
-	lua_State* GetLuaState() { return lua_state; }
-
-	void tolua__set_instance(lua_State* L, lua_Object lo) {
-
-		lua_state = L;
-
-		lua_pushvalue(L, lo);
-		lua_instance = lua_ref(lua_state, 1);
-	};
-
-	ToluaBase() {
-
-		lua_state = NULL;
-	};
-
-	~ToluaBase() {
-
-		if (lua_state) {
-
-			lua_unref(lua_state, lua_instance);
-		};
-	};
-};
-
-#endif
-
-