Use callbacks for handling character death and respawn

Rather than relying on the availability of global functions with certain
predefined names, the Lua script now calls API functions to set which
function should be called on these global events.

This mechanism should make it easier to avoid name collisions in the global
namespace, which is important now that there is only a single script state.
For these global events this was not likely to become a problem, but this
solution can also be used for callbacks on specific item or monster types,
or even allow setting callbacks on certain instances.

Reviewed-by: Erik Schilling
Reviewed-by: Yohann Ferreira

10 files changed, 125 insertions, 33 deletions

diff --git a/example/scripts/global_events.lua b/example/scripts/global_events.lua
index fe4175b..c320d16 100644
--- a/example/scripts/global_events.lua
+++ b/example/scripts/global_events.lua
@@ -10,24 +10,29 @@
 
 --]]
 
-
--- This function is called when the hit points of a character reach zero.
-function on_chr_death(ch)
+-- Register the callback that is called when the hit points of a character
+-- reach zero.
+mana.on_character_death(function(ch)
     mana.being_say(ch, "Noooooo!!!")
-end
+end)
 
 -- This function is called when the player clicks on the OK button after the
 -- death message appeared. It should be used to implement the respawn
 -- mechanic (for example: warp the character to the respawn location and
 -- bring HP above zero in some way)
-function on_chr_death_accept(ch)
+mana.on_character_death_accept(function(ch)
     -- restores to full hp
     mana.being_heal(ch)
     -- restores 1 hp (in case you want to be less nice)
     -- mana.being_heal(ch, 1)
     -- warp the character to the respawn location
     mana.chr_warp(ch, 1, 815, 100)
-end
+end)
+
+
+--
+-- TODO: All of the below functions are not implemented yet!
+--
 
 -- This function is called after chr_death_accept. The difference is that
 -- it is called in the context of the map the character is spawned on after
diff --git a/src/game-server/character.cpp b/src/game-server/character.cpp
index 819da1c..b505176 100644
--- a/src/game-server/character.cpp
+++ b/src/game-server/character.cpp
@@ -52,6 +52,24 @@ const float Character::EXPCURVE_FACTOR = 10.0f;
 const float Character::LEVEL_SKILL_PRECEDENCE_FACTOR = 0.75f;
 const float Character::EXP_LEVEL_FLEXIBILITY = 1.0f;
 
+Script::Ref Character::mDeathCallback = Script::NoRef;
+Script::Ref Character::mDeathAcceptedCallback = Script::NoRef;
+
+static bool executeCallback(Script::Ref function, Character *character)
+{
+    if (function == Script::NoRef)
+        return false;
+
+    Script *script = ScriptManager::currentState();
+    script->setMap(character->getMap());
+    script->prepare(function);
+    script->push(character);
+    script->execute();
+    script->setMap(0);
+    return true;
+}
+
+
 Character::Character(MessageIn &msg):
     Being(OBJECT_CHARACTER),
     mClient(NULL),
@@ -197,7 +215,7 @@ void Character::perform()
 void Character::died()
 {
     Being::died();
-    ScriptManager::executeGlobalEventFunction("on_chr_death", this);
+    executeCallback(mDeathCallback, this);
 }
 
 void Character::respawn()
@@ -214,11 +232,11 @@ void Character::respawn()
     // Reset target
     mTarget = NULL;
 
-    // Execute respawn script
-    if (ScriptManager::executeGlobalEventFunction("on_chr_death_accept", this))
+    // Execute respawn callback when set
+    if (executeCallback(mDeathAcceptedCallback, this))
         return;
 
-    // Script-controlled respawning didn't work - fall back to hardcoded logic.
+    // No script respawn callback set - fall back to hardcoded logic
     mAttributes[ATTR_HP].setBase(mAttributes[ATTR_MAX_HP].getModifiedAttribute());
     updateDerivedAttributes(ATTR_HP);
     // Warp back to spawn point.
diff --git a/src/game-server/character.h b/src/game-server/character.h
index 7777217..7649842 100644
--- a/src/game-server/character.h
+++ b/src/game-server/character.h
@@ -26,9 +26,10 @@
 #include <vector>
 
 #include "common/defines.h"
-#include "common/manaserv_protocol.h"
 #include "common/inventorydata.h"
+#include "common/manaserv_protocol.h"
 #include "game-server/being.h"
+#include "scripting/script.h"
 #include "utils/logger.h"
 
 class BuySell;
@@ -361,6 +362,12 @@ class Character : public Being
         bool isConnected() const
         { return mConnected; }
 
+        static void setDeathCallback(Script *script)
+        { script->assignCallback(mDeathCallback); }
+
+        static void setDeathAcceptedCallback(Script *script)
+        { script->assignCallback(mDeathAcceptedCallback); }
+
     protected:
         /**
          * Gets the way the actor blocks pathfinding for other objects
@@ -458,6 +465,9 @@ class Character : public Being
         TransactionType mTransaction; /**< Trade/buy/sell action the character is involved in. */
         std::map<int, int> mKillCount;  /**< How many monsters the character has slain of each type */
 
+        static Script::Ref mDeathCallback;
+        static Script::Ref mDeathAcceptedCallback;
+
         // Set as a friend, but still a lot of redundant accessors. FIXME.
         template< class T >
         friend void serializeCharacterData(const T &data, MessageOut &msg);
diff --git a/src/scripting/lua.cpp b/src/scripting/lua.cpp
index 9aec6b3..7a94ce2 100644
--- a/src/scripting/lua.cpp
+++ b/src/scripting/lua.cpp
@@ -74,6 +74,30 @@ static Script *getScript(lua_State *s)
 
 
 /**
+ * mana.on_character_death( function(Character*) ): void
+ * Sets a listener function to the character death event.
+ */
+static int on_character_death(lua_State *s)
+{
+    luaL_checktype(s, 1, LUA_TFUNCTION);
+    Character::setDeathCallback(getScript(s));
+    return 0;
+}
+
+/**
+ * mana.on_character_death_accept( function(Character*) ): void
+ * Sets a listener function that is called when the player clicks on the OK
+ * button after the death message appeared. It should be used to implement the
+ * respawn mechanic.
+ */
+static int on_character_death_accept(lua_State *s)
+{
+    luaL_checktype(s, 1, LUA_TFUNCTION);
+    Character::setDeathAcceptedCallback(getScript(s));
+    return 0;
+}
+
+/**
  * mana.npc_message(NPC*, Character*, string): void
  * Callback for sending a NPC_MESSAGE.
  */
@@ -2564,6 +2588,8 @@ LuaScript::LuaScript():
 
     // Put the callback functions in the scripting environment.
     static luaL_Reg const callbacks[] = {
+        { "on_character_death",              &on_character_death              },
+        { "on_character_death_accept",       &on_character_death_accept       },
         { "npc_create",                      &npc_create                      },
         { "npc_message",                     &npc_message                     },
         { "npc_choice",                      &npc_choice                      },
diff --git a/src/scripting/luascript.cpp b/src/scripting/luascript.cpp
index c487aa6..605302e 100644
--- a/src/scripting/luascript.cpp
+++ b/src/scripting/luascript.cpp
@@ -35,6 +35,15 @@ LuaScript::~LuaScript()
     lua_close(mState);
 }
 
+void LuaScript::prepare(Ref function)
+{
+    assert(nbArgs == -1);
+    lua_rawgeti(mState, LUA_REGISTRYINDEX, function);
+    assert(lua_isfunction(mState, -1));
+    nbArgs = 0;
+    mCurFunction = "<callback>"; // We don't know the function name
+}
+
 void LuaScript::prepare(const std::string &name)
 {
     assert(nbArgs == -1);
@@ -108,6 +117,17 @@ int LuaScript::execute()
     mCurFunction.clear();
 }
 
+void LuaScript::assignCallback(Script::Ref &function)
+{
+    assert(lua_isfunction(mState, -1));
+
+    // If there is already a callback set, replace it
+    if (function != NoRef)
+        luaL_unref(mState, LUA_REGISTRYINDEX, function);
+
+    function = luaL_ref(mState, LUA_REGISTRYINDEX);
+}
+
 void LuaScript::load(const char *prog, const char *name)
 {
     int res = luaL_loadbuffer(mState, prog, std::strlen(prog), name);
diff --git a/src/scripting/luascript.h b/src/scripting/luascript.h
index 0d59703..6f2bcee 100644
--- a/src/scripting/luascript.h
+++ b/src/scripting/luascript.h
@@ -44,6 +44,8 @@ class LuaScript : public Script
 
         void load(const char *prog, const char *name);
 
+        void prepare(Ref function);
+
         void prepare(const std::string &);
 
         void push(int);
@@ -56,6 +58,8 @@ class LuaScript : public Script
 
         int execute();
 
+        void assignCallback(Ref &function);
+
         static void getQuestCallback(Character *, const std::string &,
                                      const std::string &, void *);
 
diff --git a/src/scripting/script.cpp b/src/scripting/script.cpp
index db44bd5..6672084 100644
--- a/src/scripting/script.cpp
+++ b/src/scripting/script.cpp
@@ -34,6 +34,8 @@ typedef std::map< std::string, Script::Factory > Engines;
 
 static Engines *engines = NULL;
 
+Script::Ref Script::NoRef = -1;
+
 Script::Script():
     mMap(NULL),
     mEventListener(&scriptEventDispatch)
diff --git a/src/scripting/script.h b/src/scripting/script.h
index bd14311..b475a0f 100644
--- a/src/scripting/script.h
+++ b/src/scripting/script.h
@@ -21,11 +21,12 @@
 #ifndef SCRIPTING_SCRIPT_H
 #define SCRIPTING_SCRIPT_H
 
-#include <string>
-
-#include "game-server/character.h"
+#include "common/inventorydata.h"
 #include "game-server/eventlistener.h"
 
+#include <list>
+#include <string>
+
 class MapComposite;
 class Thing;
 
@@ -35,7 +36,9 @@ class Thing;
 class Script
 {
     public:
-
+        /**
+         * Defines a function that creates a Script instance.
+         */
         typedef Script *(*Factory)();
 
         /**
@@ -48,6 +51,15 @@ class Script
          */
         static Script *create(const std::string &engine);
 
+        /**
+         * A reference to a script object. It's just an integer, but the
+         * typedef makes the purpose of the variable clear.
+         *
+         * Variables of this type should be initialized to Script::NoRef.
+         */
+        typedef int Ref;
+        static Ref NoRef;
+
         Script();
 
         virtual ~Script() {}
@@ -81,6 +93,12 @@ class Script
         virtual void update();
 
         /**
+         * Prepares a call to the referenced function.
+         * Only one function can be prepared at once.
+         */
+        virtual void prepare(Ref function) = 0;
+
+        /**
          * Prepares a call to the given function.
          * Only one function can be prepared at once.
          */
@@ -117,6 +135,13 @@ class Script
         virtual int execute() = 0;
 
         /**
+         * Assigns the current callback to the given \a function.
+         *
+         * Where the callback exactly comes from is up to the script engine.
+         */
+        virtual void assignCallback(Ref &function) = 0;
+
+        /**
          * Sets associated map.
          */
         void setMap(MapComposite *m)
diff --git a/src/scripting/scriptmanager.cpp b/src/scripting/scriptmanager.cpp
index 5251569..d58379f 100644
--- a/src/scripting/scriptmanager.cpp
+++ b/src/scripting/scriptmanager.cpp
@@ -49,19 +49,6 @@ Script *ScriptManager::currentState()
 
 // TODO: Have some generic event mechanism rather than calling global functions
 
-bool ScriptManager::executeGlobalEventFunction(const std::string &function, Being* obj)
-{
-    bool isScriptHandled = false;
-    _currentState->setMap(obj->getMap());
-    _currentState->prepare(function);
-    _currentState->push(obj);
-    _currentState->execute();
-    _currentState->setMap(NULL);
-    isScriptHandled = true; // TODO: don't set to true when execution failed
-    return isScriptHandled;
-}
-
-
 void ScriptManager::addDataToSpecial(int id, Special *special)
 {
     /* currently only gets the recharge cost.
diff --git a/src/scripting/scriptmanager.h b/src/scripting/scriptmanager.h
index 7560c34..ade76c1 100644
--- a/src/scripting/scriptmanager.h
+++ b/src/scripting/scriptmanager.h
@@ -56,11 +56,6 @@ bool loadMainScript(const std::string &file);
  */
 Script *currentState();
 
-
-/**
- * Runs a global function from the global event script file
- */
-bool executeGlobalEventFunction(const std::string &function, Being *obj);
 void addDataToSpecial(int specialId, Special *special);
 bool performSpecialAction(int specialId, Being *caster);
 bool performCraft(Being *crafter, const std::list<InventoryItem> &recipe);