Merge pull request cuberite#1960 from mc-server/FixInterPluginCalls

Fix inter plugin calls

CMakeLists.txt

@@ -78,6 +78,9 @@ endif()
 # The Expat library is linked in statically, make the source files aware of that:
 add_definitions(-DXML_STATIC)
 
+# Let Lua use additional checks on its C API. This is only compiled into Debug builds:
+add_definitions(-DLUA_USE_APICHECK)
+
 # Self Test Mode enables extra checks at startup
 if(${SELF_TEST})
 	add_definitions(-DSELF_TEST)

MCServer/Plugins/Debuggers/Debuggers.lua

@@ -54,7 +54,7 @@ function Initialize(a_Plugin)
 	-- TestBlockAreas()
 	-- TestSQLiteBindings()
 	-- TestExpatBindings()
-	-- TestPluginCalls()
+	TestPluginCalls()
 
 	TestBlockAreasString()
 	TestStringBase64()
@@ -157,26 +157,18 @@ function TestPluginCalls()
 	-- The Split parameter should be a table, but it is not used in that function anyway,
 	-- so we can get away with passing nil to it.
 
-	-- Use the old, deprecated and unsafe method:
-	local Core = cPluginManager:Get():GetPlugin("Core")
-	if (Core ~= nil) then
-		LOGINFO("Calling Core::ReturnColorFromChar() the old-fashioned way...")
-		local Gray = Core:Call("ReturnColorFromChar", nil, "8")
-		if (Gray ~= cChatColor.Gray) then
-			LOGWARNING("Call failed, exp " .. cChatColor.Gray .. ", got " .. (Gray or "<nil>"))
-		else
-			LOGINFO("Call succeeded")
-		end
-	end
-
-	-- Use the new method:
-	LOGINFO("Calling Core::ReturnColorFromChar() the recommended way...")
-	local Gray = cPluginManager:CallPlugin("Core", "ReturnColorFromChar", nil, "8")
+	LOG("Debuggers: Calling NoSuchPlugin.FnName()...")
+	cPluginManager:CallPlugin("NoSuchPlugin", "FnName", "SomeParam")
+	LOG("Debuggers: Calling Core.NoSuchFunction()...")
+	cPluginManager:CallPlugin("Core", "NoSuchFunction", "SomeParam")
+	LOG("Debuggers: Calling Core.ReturnColorFromChar(..., \"8\")...")
+	local Gray = cPluginManager:CallPlugin("Core", "ReturnColorFromChar", "split", "8")
 	if (Gray ~= cChatColor.Gray) then
-		LOGWARNING("Call failed, exp " .. cChatColor.Gray .. ", got " .. (Gray or "<nil>"))
+		LOGWARNING("Debuggers: Call failed, exp " .. cChatColor.Gray .. ", got " .. (Gray or "<nil>"))
 	else
-		LOGINFO("Call succeeded")
+		LOG("Debuggers: Call succeeded")
 	end
+	LOG("Debuggers: Inter-plugin calls done.")
 end
 
 

src/Bindings/LuaState.cpp

@@ -1535,15 +1535,15 @@ int cLuaState::CallFunctionWithForeignParams(
 	if (!PushFunction(a_FunctionName.c_str()))
 	{
 		LOGWARNING("Function '%s' not found", a_FunctionName.c_str());
-		lua_pop(m_LuaState, 2);
+		lua_settop(m_LuaState, OldTop);
 		return -1;
 	}
 
 	// Copy the function parameters to the target state
 	if (CopyStackFrom(a_SrcLuaState, a_SrcParamStart, a_SrcParamEnd) < 0)
 	{
 		// Something went wrong, fix the stack and exit
-		lua_pop(m_LuaState, 2);
+		lua_settop(m_LuaState, OldTop);
 		m_NumCurrentFunctionArgs = -1;
 		m_CurrentFunctionName.clear();
 		return -1;
@@ -1554,13 +1554,8 @@ int cLuaState::CallFunctionWithForeignParams(
 	if (ReportErrors(s))
 	{
 		LOGWARN("Error while calling function '%s' in '%s'", a_FunctionName.c_str(), m_SubsystemName.c_str());
-		// Fix the stack.
-		// We don't know how many values have been pushed, so just get rid of any that weren't there initially
-		int CurTop = lua_gettop(m_LuaState);
-		if (CurTop > OldTop)
-		{
-			lua_pop(m_LuaState, CurTop - OldTop);
-		}
+		// Reset the stack:
+		lua_settop(m_LuaState, OldTop);
 
 		// Reset the internal checking mechanisms:
 		m_NumCurrentFunctionArgs = -1;

src/Bindings/ManualBindings.cpp

@@ -1988,6 +1988,11 @@ static int tolua_cPluginManager_CallPlugin(lua_State * tolua_S)
 	{
 		return 0;
 	}
+	if (Callback.m_NumReturns < 0)
+	{
+		// The call has failed, there are zero return values. Do NOT return negative number (Lua considers that a "yield")
+		return 0;
+	}
 	return Callback.m_NumReturns;
 }