better function registration: store a reference, instead of making up a name

stepmania/src/ActorCommands.cpp

@@ -12,18 +12,12 @@ template<>
 set<ActorCommands*>* SubscriptionManager<ActorCommands>::s_pSubscribers = NULL;
 
 
-static CString GetNextFunctionName()
-{
-	static int id = 0;
-	++id;
-	return ssprintf( "F%08x",id );
-}
-
 ActorCommands::ActorCommands( const Commands& cmds )
 {
 	SubscriptionManager<ActorCommands>::Subscribe( this );
 	
 	m_cmds = cmds;
+	m_iLuaFunction = LUA_NOREF;
 
 	Register();
 }
@@ -32,24 +26,16 @@ ActorCommands::~ActorCommands()
 {
 	SubscriptionManager<ActorCommands>::Unsubscribe( this );
 
-	if( m_sLuaFunctionName.size() )
+	Unregister();
-		Unregister();
 }
 
 ActorCommands::ActorCommands( const ActorCommands& cpy )
 {
 	SubscriptionManager<ActorCommands>::Subscribe( this );
 
-	m_sLuaFunctionName = GetNextFunctionName();
+	/* Make a new reference. */
-
+	lua_rawgeti( LUA->L, LUA_REGISTRYINDEX, cpy.m_iLuaFunction );
-	/* We need to make a new function, since we'll be unregistered separately.  Set
+	m_iLuaFunction = luaL_ref( LUA->L, LUA_REGISTRYINDEX );
-	 * the function by reference, so we don't make a new function unless we're actually
-	 * changed. */
-	ostringstream s;
-	s << m_sLuaFunctionName << " = " << cpy.m_sLuaFunctionName;
-
-	CString s2 = s.str();
-	LUA->RunScript( s2 );
 }
 
 ActorCommands &ActorCommands::operator=( const ActorCommands& cpy )
@@ -63,31 +49,30 @@ ActorCommands &ActorCommands::operator=( const ActorCommands& cpy )
 
 void ActorCommands::PushSelf( lua_State *L ) const
 {
-	lua_pushstring( L, m_sLuaFunctionName ); // function name
+	ASSERT( m_iLuaFunction != LUA_NOREF );
-	lua_gettable( L, LUA_GLOBALSINDEX ); // function to be called
 
-	ASSERT_M( !lua_isnil(L, -1), m_sLuaFunctionName.c_str() )
+	if( m_iLuaFunction != LUA_REFNIL )
+		lua_rawgeti( LUA->L, LUA_REGISTRYINDEX, m_iLuaFunction );
+	else
+		LUA->PushNopFunction();
+
+	ASSERT_M( !lua_isnil(L, -1), ssprintf("%i", m_iLuaFunction) )
 }
 
 void ActorCommands::Register()
 {
 	if( m_cmds.v.size() == 0 )
 	{
-		m_sLuaFunctionName = "nop";
+		m_iLuaFunction = LUA_REFNIL;
 		return;
 	}
 
-	// TODO: calculate a better function name, or figure out how
-	// to keep a pointer directly to the Lua function so no global
-	// table lookup is necessary.
-	m_sLuaFunctionName = GetNextFunctionName();
-
 	//
 	// Convert cmds to a Lua function
 	//
 	ostringstream s;
 	
-	s << m_sLuaFunctionName << " = function(self)\n";
+	s << "return function(self)\n";
 
 	FOREACH_CONST( Command, m_cmds.v, c )
 	{
@@ -139,7 +124,10 @@ void ActorCommands::Register()
 
 
 	CString s2 = s.str();
-	LUA->RunScript( s2 );
+	LUA->RunScript( s2, 1 );
+
+	/* The function is now on the stack. */
+	m_iLuaFunction = luaL_ref( LUA->L, LUA_REGISTRYINDEX );
 }
 
 void ActorCommands::Unregister()
@@ -147,24 +135,26 @@ void ActorCommands::Unregister()
 	if( LUA == NULL )
 		return;	// nothing to do
 
-	ASSERT( m_sLuaFunctionName.size() );
+	luaL_unref( LUA->L, LUA_REGISTRYINDEX, m_iLuaFunction );
-
+	m_iLuaFunction = LUA_NOREF;
-	if( m_sLuaFunctionName != "nop" )
-	{
-		lua_pushstring( LUA->L, m_sLuaFunctionName );
-		lua_pushnil( LUA->L );
-		lua_settable( LUA->L, LUA_GLOBALSINDEX );
-	}
-
-	m_sLuaFunctionName = "";
 }
 
+void ActorCommands::ReRegister()
+{
+	/* When called, the Lua state has been wiped.  Don't try to unregister our old
+	 * function reference, since it's already gone (and the number may point somewhere
+	 * else). */
+	m_iLuaFunction = LUA_NOREF;
+	Register();
+}
+
+
 void ActorCommands::ReRegisterAll()
 {
 	if( SubscriptionManager<ActorCommands>::s_pSubscribers == NULL )
 		return;
 	FOREACHS( ActorCommands*, *SubscriptionManager<ActorCommands>::s_pSubscribers, p )
-		(*p)->Register();
+		(*p)->ReRegister();
 }
 
 

stepmania/src/ActorCommands.h

@@ -22,10 +22,11 @@ public:
 
 private:
 	void Register();
+	void ReRegister();
 	void Unregister();
 
 	Commands m_cmds;
-	CString m_sLuaFunctionName;
+	int m_iLuaFunction;
 };
 
 typedef AutoPtrCopyOnWrite<ActorCommands> apActorCommands;

stepmania/src/LuaFunctions.h

@@ -8,6 +8,7 @@ extern "C"
 {
 #include <lua.h>
 #include <lualib.h>
+#include <lauxlib.h>
 }
 
 /* Argument helpers: */

stepmania/src/LuaManager.cpp

@@ -59,6 +59,13 @@ void LuaManager::PushStackNil()
 	lua_pushnil( L );
 }
 
+void LuaManager::PushNopFunction()
+{
+	lua_rawgeti( LUA->L, LUA_REGISTRYINDEX, m_iNopFunction );
+
+	ASSERT_M( !lua_isnil(L, -1), ssprintf("%i", m_iNopFunction) )
+}
+
 void LuaManager::PushStack( int out, lua_State *L )
 {
 	if( L == NULL )
@@ -185,7 +192,9 @@ void LuaManager::ResetState()
 	luaopen_string( L );
 	lua_settop(L, 0); // luaopen_* pushes stuff onto the stack that we don't need
 
-	RunScript( "nop = function(self) end" );
+	/* Set up the NOP function pointer. */
+	RunScript( "return function() end", 1 );
+	m_iNopFunction = luaL_ref( L, LUA_REGISTRYINDEX );
 
 	for( const LuaFunctionList *p = g_LuaFunctions; p; p=p->next )
 		lua_register( L, p->name, p->func );
@@ -235,7 +244,7 @@ bool LuaManager::RunScriptFile( const CString &sFile )
 	return RunScript( sScript );
 }
 
-bool LuaManager::RunScript( const CString &sScript )
+bool LuaManager::RunScript( const CString &sScript, int iReturnValues )
 {
 	// load string
 	{
@@ -257,7 +266,7 @@ bool LuaManager::RunScript( const CString &sScript )
 
 	// evaluate
 	{
-		int ret = lua_pcall(L, 0, 0, 0);
+		int ret = lua_pcall( L, 0, iReturnValues, 0 );
 		if( ret )
 		{
 			CString err;

stepmania/src/LuaManager.h

@@ -22,7 +22,7 @@ public:
 	void ResetState();
 
 	/* Run a complete script in the global environment, which returns no value. */
-	bool RunScript( const CString &sScript );
+	bool RunScript( const CString &sScript, int iReturnValues = 0 );
 
 	/* Run an expression in the global environment, returning the given type. */
 	bool RunExpressionB( const CString &str );
@@ -39,6 +39,7 @@ public:
 	void UnsetGlobal( const CString &sName ) { PushStackNil(); SetGlobal( sName ); }
 
 	void PushStackNil();
+	void PushNopFunction();
 	static void PushStack( bool val, lua_State *L = NULL );
 	static void PushStack( float val, lua_State *L = NULL );
 	static void PushStack( int val, lua_State *L = NULL );
@@ -53,6 +54,7 @@ public:
 	bool RunExpression( const CString &str );
 	lua_State *L;
 private:
+	int m_iNopFunction;
 };
 
 extern LuaManager *LUA;