Merge pull request #272 from FreeZoneMods/xd_dev

Improve script reset notifying system

Author: Xottab-DUTY

src/utils/xrSE_Factory/ai_space.cpp

@@ -11,11 +11,35 @@
 #include "xrScriptEngine/script_engine.hpp"
 #include "xrServerEntities/object_factory.h"
 
-static CAI_Space g_ai_space;
+// Nifty counter required to solve initilization order problem
+// Please, do NOT initialize this static members directly, it breaks the initialization logic
+// And yes, according to the standard ($3.6.2/1): "Objects with static storage duration (3.7.1) shall be
+// zero-initialized (8.5) before any other initialization takes place" So, it must be automatically
+// initialized with zeros before executing the constructor
+static CAI_Space* s_ai_space;
+static u32 s_nifty_counter;
+
+SAI_Space_Initializer::SAI_Space_Initializer()
+{
+    if (s_nifty_counter++ == 0)
+    {
+        s_ai_space = new CAI_Space();
+    }
+}
+
+SAI_Space_Initializer::~SAI_Space_Initializer()
+{
+    if (--s_nifty_counter == 0)
+    {
+        xr_delete(s_ai_space);
+    }
+}
 
 CAI_Space& CAI_Space::GetInstance()
 {
-    auto& instance = g_ai_space;
+    VERIFY(s_ai_space);
+
+    auto& instance = *s_ai_space;
     if (!instance.m_inited)
     {
         instance.init();

src/utils/xrSE_Factory/ai_space.h

@@ -34,10 +34,24 @@ class CAI_Space
         EVENT_SCRIPT_ENGINE_RESET,
         EVENT_COUNT,
     };
-    CEventNotifierCallback::CID Subscribe(CEventNotifierCallback* cb, EEventID event_id) { return 0; }
+
+    template <class CB, class... Args>
+    CEventNotifierCallback::CID Subscribe(EEventID event_id, Args&&... args)
+    {
+        return 0;
+    }
+
     bool Unsubscribe(CEventNotifierCallback::CID cid, EEventID event_id) { return true; }
 };
 
+// Static initializer for every translation unit
+// Need to avoid static initilization order problem
+static struct SAI_Space_Initializer
+{
+    SAI_Space_Initializer();
+    ~SAI_Space_Initializer();
+} s_AISpaceInitializer;
+
 IC CAI_Space& ai();
 
 #include "ai_space_inline.h"

src/xrCore/Events/Notifier.h

@@ -1,3 +1,10 @@
+/*!
+\file
+\brief Implementation of simple thread-safe event processing and notifying system
+
+\details Will be useful when you need a system which allows you to handle events with calling the subscribed callbacks.
+*/
+
 #pragma once
 
 #include "xrCommon/xr_smart_pointers.h"
@@ -6,16 +13,52 @@
 #include "xrCore/Threading/Lock.hpp"
 #include "xrCore/Threading/ScopeLock.hpp"
 
+/*!
+    \brief Base abstract class for implementing event handling callbacks
+
+    \details Derive it and override @ref ProcessEvent method, then register the instanciated callback into the event
+   notifier. The @ref ProcessEvent method will be called by the notifier every time when the selected event happens
+*/
 class CEventNotifierCallback
 {
 public:
+    /*! Callback ID, type for identifying registered callbacks in the notifier */
     using CID = size_t;
+
+    /*! Invalid callback ID, will be never returned after successful subscription */
     static const CID INVALID_CID = std::numeric_limits<CID>::max();
 
+    /*! This method will be automatically called by the notifier for processing the event */
     virtual void ProcessEvent() = 0;
+
     virtual ~CEventNotifierCallback(){};
 };
 
+/*!
+    \brief Abstract class for event processing callback which stores own callback ID
+
+    \details Designed to use primarily in pair with @ref CEventNotifier::CreateRegisteredCallback method,
+     very useful for self-unsubscribing callbacks
+*/
+class CEventNotifierCallbackWithCid : public CEventNotifierCallback
+{
+private:
+    const CID m_cid;
+
+public:
+    /*! Constructor, takes callback ID which was generated by the notifier
+        /param[in] cid callback ID, should be generated by the notifier in subscription process only
+    */
+    CEventNotifierCallbackWithCid(CID cid) : m_cid(cid), CEventNotifierCallback(){};
+
+    /*! Returns the callback ID which was generated by the notifier and passed to the constructor */
+    CID GetCid() const { return m_cid; }
+};
+
+/*! \brief Template class for the event processing dispatcher
+    \details Manages with subscribing, calling and unsubscribing handlers. Template parameter CNT is a count of events
+    which we are going to process using the dispatcher. Owns the event's handling callbacks and controls its lifetime.
+*/
 template <unsigned int CNT>
 class CEventNotifier
 {
@@ -41,6 +84,14 @@ class CEventNotifier
         xr_vector<CCallbackWrapper> m_callbacks;
         Lock m_lock;
 
+        CEventNotifierCallback::CID FindFreeCid()
+        {
+            ScopeLock lock(&m_lock);
+            auto it = std::find(m_callbacks.begin(), m_callbacks.end(), nullptr);
+            return (it == m_callbacks.end()) ? CEventNotifierCallback::INVALID_CID :
+                                               std::distance(m_callbacks.begin(), it);
+        }
+
     public:
         CEventNotifierCallback::CID RegisterCallback(CEventNotifierCallback* cb)
         {
@@ -50,6 +101,28 @@ class CEventNotifier
                                                (it->callback.reset(cb), std::distance(m_callbacks.begin(), it));
         }
 
+        template <class CB, class... Args>
+        CEventNotifierCallback::CID CreateRegisteredCallback(Args&&... args)
+        {
+            static_assert(std::is_base_of<CEventNotifierCallbackWithCid, CB>::value);
+
+            ScopeLock lock(&m_lock);
+
+            auto cid = FindFreeCid();
+            CB* cb = new CB((cid == CEventNotifierCallback::INVALID_CID) ? m_callbacks.size() : cid, args...);
+
+            if (cid == CEventNotifierCallback::INVALID_CID)
+            {
+                m_callbacks.emplace_back(cb);
+            }
+            else
+            {
+                m_callbacks[cid].callback.reset(cb);
+            }
+
+            return cb->GetCid();
+        }
+
         bool UnregisterCallback(CEventNotifierCallback::CID cid)
         {
             bool result = false;
@@ -94,18 +167,67 @@ class CEventNotifier
     xr_array<CCallbackStorage, CNT> m_callbacks;
 
 public:
+    /*! \brief Method for registering an existing event handler.
+
+        \details Use it to register in the notifier previously created event handling callback.
+        \warning Please consider using @ref CreateRegisteredCallback method instead. When using the @ref
+        RegisterCallback method you should register only the callbacks which were created by the other methods of the
+        notifier. This restriction happens because the notifier owns the callbacks and destroyes it internally, so you
+        can get some problems when creating the callbacks outside of the notifier (notifier will call improper
+        destruction methods).
+        \todo Provide the method for creating callbacks without the registration
+        \param[in] cb pointer to
+        the callback to register. After registration the notifier becomes an owner of the registered callback
+        \param[in] event_id Index of the event type which the callback should process, must be lesser than the template
+        parameter CNT
+        \return Callback ID of the registered callback (unique for each event type)
+    */
     CEventNotifierCallback::CID RegisterCallback(CEventNotifierCallback* cb, unsigned int event_id)
     {
         R_ASSERT(event_id < CNT);
         return m_callbacks[event_id].RegisterCallback(cb);
     }
 
+    /*! \brief Template method which creates a new event handling callback and registers it in the notifier
+
+        \details Provides a convenient way for registering the event handlers. It's a preferred method for setting up
+        the event handlers. The template  parameter CB is a type of the event handling callback which we are going to
+        create
+
+        \param[in] event_id Index of the event type which the callback should process, must be lesser than template
+        parameter template parameter CNT
+        \param[in] args Arguments which should be passed to the constructor of the callback
+        \return Callback ID of the registered callback (unique for each event type)
+    */
+    template <class CB, class... Args>
+    CEventNotifierCallback::CID CreateRegisteredCallback(unsigned int event_id, Args&&... args)
+    {
+        R_ASSERT(event_id < CNT);
+        return m_callbacks[event_id].CreateRegisteredCallback<CB>(args...);
+    }
+
+    /*! \brief Provides the way to unsubscribe and delete the callback
+
+        \details The callback will be automatically destroyed by the notifier after unsubscribing. However, the
+        unsubscribing logic allows to unsubscribe the callback directly while processing the event. Callback won't be
+       destroyed while its event handling method is executing.
+
+       \param[in] cid Callback ID which was returning by the subscribing function
+       \param[in] event_id Index of the event type which the callback is subscribed to
+       \return True if unsubscribing is successful, false otherwise
+    */
     bool UnregisterCallback(CEventNotifierCallback::CID cid, unsigned int event_id)
     {
         R_ASSERT(event_id < CNT);
         return m_callbacks[event_id].UnregisterCallback(cid);
     }
 
+    /*! \brief The method to notify the subscribed callbacks about the event
+
+        \details Call this method when the event happens. All the subscribed callbacks would be called one-by-one in the
+        caller's thread.
+        \param[in] event_id Index of the event type
+    */
     void FireEvent(unsigned int event_id)
     {
         R_ASSERT(event_id < CNT);

src/xrGame/MainMenu.cpp

@@ -67,16 +67,16 @@ CMainMenu* MainMenu() { return (CMainMenu*)g_pGamePersistent->m_pMainMenu; };
 
 CMainMenu::CMainMenu()
 {
-    class CResetEventCb : public CEventNotifierCallback
+    class CResetEventCb : public CEventNotifierCallbackWithCid
     {
         CMainMenu* m_mainmenu;
 
     public:
-        CResetEventCb(CMainMenu* mm) : m_mainmenu(mm) {}
+        CResetEventCb(CID cid, CMainMenu* mm) : m_mainmenu(mm), CEventNotifierCallbackWithCid(cid) {}
         void ProcessEvent() override { m_mainmenu->DestroyInternal(true); }
     };
 
-    m_script_reset_event_cid = ai().Subscribe(new CResetEventCb(this), CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
+    m_script_reset_event_cid = ai().Subscribe<CResetEventCb>(CAI_Space::EVENT_SCRIPT_ENGINE_RESET, this);
 
     m_Flags.zero();
     m_startDialog = NULL;

src/xrGame/ai_space.cpp

@@ -22,11 +22,35 @@
 #include "moving_objects.h"
 #include "doors_manager.h"
 
-static CAI_Space g_ai_space;
+// Nifty counter required to solve initilization order problem
+// Please, do NOT initialize this static members directly, it breaks the initialization logic
+// And yes, according to the standard ($3.6.2/1): "Objects with static storage duration (3.7.1) shall be
+// zero-initialized (8.5) before any other initialization takes place" So, it must be automatically
+// initialized with zeros before executing the constructor
+static CAI_Space* s_ai_space;
+static u32 s_nifty_counter;
+
+SAI_Space_Initializer::SAI_Space_Initializer()
+{
+    if (s_nifty_counter++ == 0)
+    {
+        s_ai_space = new CAI_Space();
+    }
+}
+
+SAI_Space_Initializer::~SAI_Space_Initializer()
+{
+    if (--s_nifty_counter == 0)
+    {
+        xr_delete(s_ai_space);
+    }
+}
 
 CAI_Space& CAI_Space::GetInstance()
 {
-    auto& instance = g_ai_space;
+    VERIFY(s_ai_space);
+
+    auto& instance = *s_ai_space;
     if (!instance.m_inited)
     {
         instance.init();
@@ -197,11 +221,6 @@ void CAI_Space::set_alife(CALifeSimulator* alife_simulator)
     SetGameGraph(nullptr);
 }
 
-CEventNotifierCallback::CID CAI_Space::Subscribe(CEventNotifierCallback* cb, EEventID event_id)
-{
-    return m_events_notifier.RegisterCallback(cb, event_id);
-}
-
 bool CAI_Space::Unsubscribe(CEventNotifierCallback::CID cid, EEventID event_id)
 {
     return m_events_notifier.UnregisterCallback(cid, event_id);

src/xrGame/ai_space.h

@@ -73,7 +73,12 @@ class CAI_Space : public AISpaceBase
     virtual ~CAI_Space();
     static CAI_Space& GetInstance();
 
-    CEventNotifierCallback::CID Subscribe(CEventNotifierCallback* cb, EEventID event_id);
+    template <class CB, class... Args>
+    CEventNotifierCallback::CID Subscribe(EEventID event_id, Args&&... args)
+    {
+        return m_events_notifier.CreateRegisteredCallback<CB>(event_id, args...);
+    }
+
     bool Unsubscribe(CEventNotifierCallback::CID cid, EEventID event_id);
     void RestartScriptEngine();
 
@@ -86,6 +91,14 @@ class CAI_Space : public AISpaceBase
     IC doors::manager& doors() const;
 };
 
+// Static initializer for every translation unit
+// Need to avoid static initilization order problem
+static struct SAI_Space_Initializer
+{
+    SAI_Space_Initializer();
+    ~SAI_Space_Initializer();
+} s_AISpaceInitializer;
+
 IC CAI_Space& ai();
 
 #include "ai_space_inline.h"

src/xrGame/ui/UIWpnParams.cpp

@@ -108,21 +108,19 @@ void CUIWpnParams::SetInfo(CInventoryItem* slot_wpn, CInventoryItem& cur_wpn)
 {
     if (!g_lua_wpn_params)
     {
-        class CResetEventCb : public CEventNotifierCallback
+        class CResetEventCb : public CEventNotifierCallbackWithCid
         {
         public:
-            CID m_cid = INVALID_CID;
-
+            CResetEventCb(CID cid) : CEventNotifierCallbackWithCid(cid) {}
             void ProcessEvent() override
             {
                 xr_delete(g_lua_wpn_params);
-                ai().Unsubscribe(m_cid, CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
+                ai().Unsubscribe(GetCid(), CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
             }
         };
 
         g_lua_wpn_params = new SLuaWpnParams();
-        auto cb = new CResetEventCb();
-        cb->m_cid = ai().Subscribe(cb, CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
+        ai().Subscribe<CResetEventCb>(CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
     }
 
     LPCSTR cur_section = cur_wpn.object().cNameSect().c_str();

src/xrServerEntities/object_factory_inline.h

@@ -18,20 +18,19 @@ IC const CObjectFactory& object_factory()
         g_object_factory = new CObjectFactory();
         g_object_factory->init();
 
-        class CResetEventCb : public CEventNotifierCallback
+        class CResetEventCb : public CEventNotifierCallbackWithCid
         {
         public:
-            CID m_cid;
+            CResetEventCb(CID cid) : CEventNotifierCallbackWithCid(cid) {}
 
             void ProcessEvent() override
             {
                 xr_delete(g_object_factory);
-                ai().Unsubscribe(m_cid, CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
+                ai().Unsubscribe(GetCid(), CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
             }
         };
 
-        CResetEventCb* e = new CResetEventCb();
-        e->m_cid = ai().Subscribe(e, CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
+        ai().Subscribe<CResetEventCb>(CAI_Space::EVENT_SCRIPT_ENGINE_RESET);
     }
     return (*g_object_factory);
 }