Use rewinder with shared_ptr for easier item destroy handling

src/items/network_item_manager.cpp

@@ -39,8 +39,7 @@ void NetworkItemManager::create()
 /** Creates a new instance of the item manager. This is done at startup
  *  of each race. */
 NetworkItemManager::NetworkItemManager()
-                  : Rewinder("N", /*can be deleted*/false),
-                    ItemManager()
+                  : Rewinder("N"), ItemManager()
 {
     m_last_confirmed_item_ticks.clear();
 

src/karts/kart_rewinder.cpp

@@ -38,8 +38,7 @@ KartRewinder::KartRewinder(const std::string& ident,
                            const btTransform& init_transform,
                            PerPlayerDifficulty difficulty,
                            std::shared_ptr<RenderInfo> ri)
-            : Rewinder(std::string("K") + StringUtils::toString(world_kart_id),
-                       /*can_be_destroyed*/ false)
+            : Rewinder(std::string("K") + StringUtils::toString(world_kart_id))
             , Kart(ident, world_kart_id, position, init_transform, difficulty,
                    ri)
 {

src/network/rewind_info.cpp

@@ -64,8 +64,9 @@ void RewindInfoState::restore()
     for (const std::string& name : m_rewinder_using)
     {
         uint16_t count = m_buffer->getUInt16();
-        Rewinder* r = RewindManager::get()->getRewinder(name);
-        if (r == NULL)
+        std::shared_ptr<Rewinder> r =
+            RewindManager::get()->getRewinder(name);
+        if (!r)
         {
             Log::error("RewindInfoState", "Missing rewinder %s",
                 name.c_str());

src/network/rewind_manager.cpp

@@ -82,18 +82,7 @@ void RewindManager::reset()
 
     if (!m_enable_rewind_manager) return;
 
-    for (auto it = m_all_rewinder.begin(); it != m_all_rewinder.end();)
-    {
-        if (!it->second->canBeDestroyed())
-        {
-            it++;
-            continue;
-        }
-        Rewinder* rewinder = it->second;
-        it = m_all_rewinder.erase(it);
-        delete rewinder;
-    }
-
+    clearExpiredRewinder();
     m_rewind_queue.reset();
 }   // reset
 
@@ -166,7 +155,9 @@ void RewindManager::saveState()
     {
         // TODO: check if it's worth passing in a sufficiently large buffer from
         // GameProtocol - this would save the copy operation.
-        BareNetworkString* buffer = p.second->saveState(&rewinder_using);
+        BareNetworkString* buffer = NULL;
+        if (auto r = p.second.lock())
+            buffer = r->saveState(&rewinder_using);
         if (buffer != NULL)
         {
             m_overall_state_size += buffer->size();
@@ -197,12 +188,16 @@ void RewindManager::update(int ticks_not_used)
     if (ticks - m_last_saved_state < m_state_frequency)
         return;
 
-    // Save state
+    // Save state, remove expired rewinder first
+    clearExpiredRewinder();
     if (NetworkConfig::get()->isClient())
     {
         auto& ret = m_local_state[ticks];
         for (auto& p : m_all_rewinder)
-            ret.push_back(p.second->getLocalStateRestoreFunction());
+        {
+            if (auto r = p.second.lock())
+                ret.push_back(r->getLocalStateRestoreFunction());
+        }
     }
     else
     {
@@ -255,6 +250,20 @@ void RewindManager::playEventsTill(int world_ticks, int *ticks)
     m_is_rewinding = false;
 }   // playEventsTill
 
+// ----------------------------------------------------------------------------
+/** Adds a Rewinder to the list of all rewinders.
+ *  \return true If successfully added, false otherwise.
+ */
+bool RewindManager::addRewinder(std::shared_ptr<Rewinder> rewinder)
+{
+    if (!m_enable_rewind_manager) return false;
+    // Maximum 1 bit to store no of rewinder used
+    if (m_all_rewinder.size() == 255)
+        return false;
+    m_all_rewinder[rewinder->getUniqueIdentity()] = rewinder;
+    return true;
+}   // addRewinder
+
 // ----------------------------------------------------------------------------
 /** Rewinds to the specified time, then goes forward till the current
  *  World::getTime() is reached again: it will replay everything before
@@ -274,7 +283,10 @@ void RewindManager::rewindTo(int rewind_ticks, int now_ticks)
     // can be computed between the transforms before and after
     // the rewind.
     for (auto& p : m_all_rewinder)
-        p.second->saveTransform();
+    {
+        if (auto r = p.second.lock())
+            r->saveTransform();
+    }
 
     // Then undo the rewind infos going backwards in time
     // --------------------------------------------------
@@ -348,7 +360,10 @@ void RewindManager::rewindTo(int rewind_ticks, int now_ticks)
 
     // Now compute the errors which need to be visually smoothed
     for (auto& p : m_all_rewinder)
-        p.second->computeError();
+    {
+        if (auto r = p.second.lock())
+            r->computeError();
+    }
 
     history->setReplayHistory(is_history);
     m_is_rewinding = false;

src/network/rewind_manager.hpp

@@ -19,7 +19,6 @@
 #ifndef HEADER_REWIND_MANAGER_HPP
 #define HEADER_REWIND_MANAGER_HPP
 
-#include "network/rewinder.hpp"
 #include "network/rewind_queue.hpp"
 #include "utils/ptr_vector.hpp"
 #include "utils/synchronised.hpp"
@@ -27,9 +26,11 @@
 #include <assert.h>
 #include <atomic>
 #include <functional>
+#include <memory>
 #include <map>
 #include <vector>
 
+class Rewinder;
 class RewindInfo;
 class EventRewinder;
 
@@ -93,7 +94,7 @@ private:
     std::map<int, std::vector<std::function<void()> > > m_local_state;
 
     /** A list of all objects that can be rewound. */
-    std::map<std::string, Rewinder*> m_all_rewinder;
+    std::map<std::string, std::weak_ptr<Rewinder> > m_all_rewinder;
 
     /** The queue that stores all rewind infos. */
     RewindQueue m_rewind_queue;
@@ -117,6 +118,19 @@ private:
 
     RewindManager();
    ~RewindManager();
+    // ------------------------------------------------------------------------
+    void clearExpiredRewinder()
+    {
+        for (auto it = m_all_rewinder.begin(); it != m_all_rewinder.end();)
+        {
+            if (it->second.expired())
+            {
+                it = m_all_rewinder.erase(it);
+                continue;
+            }
+            it++;
+        }
+    }
 
 public:
     // First static functions to manage rewinding.
@@ -151,27 +165,18 @@ public:
     void addNetworkState(BareNetworkString *buffer, int ticks);
     void saveState();
     // ------------------------------------------------------------------------
-    Rewinder* getRewinder(const std::string& name)
+    std::shared_ptr<Rewinder> getRewinder(const std::string& name)
     {
         auto it = m_all_rewinder.find(name);
-        if (it == m_all_rewinder.end())
-            return NULL;
-        return it->second;
+        if (it != m_all_rewinder.end())
+        {
+            if (auto r = it->second.lock())
+                return r;
+        }
+        return nullptr;
     }
     // ------------------------------------------------------------------------
-    /** Adds a Rewinder to the list of all rewinders.
-     *  \return true If rewinding is enabled, false otherwise. 
-     */
-    bool addRewinder(Rewinder *rewinder)
-    {
-        if (!m_enable_rewind_manager) return false;
-        // Maximum 1 bit to store no of rewinder used
-        if (m_all_rewinder.size() == 255)
-            return false;
-        m_all_rewinder[rewinder->getUniqueIdentity()] = rewinder;
-        return true;
-    }   // addRewinder
-
+    bool addRewinder(std::shared_ptr<Rewinder> rewinder);
     // ------------------------------------------------------------------------
     /** Returns true if currently a rewind is happening. */
     bool isRewinding() const { return m_is_rewinding; }

src/network/rewind_queue.cpp

@@ -21,6 +21,7 @@
 #include "config/stk_config.hpp"
 #include "modes/world.hpp"
 #include "network/network_config.hpp"
+#include "network/rewinder.hpp"
 #include "network/rewind_info.hpp"
 #include "network/rewind_manager.hpp"
 
@@ -418,9 +419,9 @@ void RewindQueue::unitTesting()
         virtual void rewind(BareNetworkString *s) {}
         virtual void saveTransform() {}
         virtual void computeError() {}
-        DummyRewinder() : Rewinder("dummy_rewinder", true) {}
+        DummyRewinder() : Rewinder() {}
     };
-    DummyRewinder *dummy_rewinder = new DummyRewinder();
+    auto dummy_rewinder = std::make_shared<DummyRewinder>();
 
     // First tests: add a state first, then an event, and make
     // sure the state stays first
@@ -434,7 +435,7 @@ void RewindQueue::unitTesting()
     assert(q0.hasMoreRewindInfo());
     assert(q0.undoUntil(0) == 0);
 
-    q0.addNetworkEvent(dummy_rewinder, NULL, 0);
+    q0.addNetworkEvent(dummy_rewinder.get(), NULL, 0);
     // Network events are not immediately merged
     assert(q0.m_all_rewind_info.size() == 1);
 
@@ -462,9 +463,9 @@ void RewindQueue::unitTesting()
     assert((*rii)->isEvent());
 
     // Test time base comparisons: adding an event to the end
-    q0.addLocalEvent(dummy_rewinder, NULL, true, 4);
+    q0.addLocalEvent(dummy_rewinder.get(), NULL, true, 4);
     // Then adding an earlier event
-    q0.addLocalEvent(dummy_rewinder, NULL, false, 1);
+    q0.addLocalEvent(dummy_rewinder.get(), NULL, false, 1);
     // rii points to the 3rd element, the ones added just now
     // should be elements4 and 5:
     rii++;

src/network/rewind_queue.hpp

@@ -19,14 +19,13 @@
 #ifndef HEADER_REWIND_QUEUE_HPP
 #define HEADER_REWIND_QUEUE_HPP
 
-#include "network/rewinder.hpp"
-#include "utils/ptr_vector.hpp"
 #include "utils/synchronised.hpp"
 
 #include <assert.h>
 #include <list>
 #include <vector>
 
+class BareNetworkString;
 class EventRewinder;
 class RewindInfo;
 class TimeStepInfo;

src/network/rewinder.cpp

@@ -20,27 +20,11 @@
 
 #include "network/rewind_manager.hpp"
 
-/** Constructor. It will add this object to the list of all rewindable
+// ----------------------------------------------------------------------------
+/** Add this object to the list of all rewindable
  *  objects in the rewind manager.
  */
-Rewinder::Rewinder(const std::string& ui, bool can_be_destroyed, bool auto_add)
-        : m_unique_identity(ui)
+bool Rewinder::add()
 {
-    assert(!m_unique_identity.empty() && m_unique_identity.size() < 255);
-    m_can_be_destroyed = can_be_destroyed;
-    if (auto_add)
-        add();
-}   // Rewinder
-
-// ----------------------------------------------------------------------------
-/** Destructor.
- */
-Rewinder::~Rewinder()
-{
-}   // ~Rewinder
-
-// ----------------------------------------------------------------------------
-void Rewinder::add()
-{
-    RewindManager::get()->addRewinder(this);
-}   // Rewinder
+    return RewindManager::get()->addRewinder(shared_from_this());
+}   // add

src/network/rewinder.hpp

@@ -19,33 +19,26 @@
 #ifndef HEADER_REWINDER_HPP
 #define HEADER_REWINDER_HPP
 
+#include <cassert>
 #include <functional>
 #include <string>
+#include <memory>
 #include <vector>
 
 class BareNetworkString;
 
-class Rewinder
+class Rewinder : public std::enable_shared_from_this<Rewinder>
 {
 protected:
-    void add();
-    // -------------------------------------------------------------------------
     void setUniqueIdentity(const std::string& uid)  { m_unique_identity = uid; }
 
 private:
     std::string m_unique_identity;
 
-    /** True if this object can be destroyed, i.e. if this object is a 'stand
-     *  alone' (i.e. not used in inheritance). If the object is used in
-     *  inheritance (e.g. KartRewinder, which is a Rewinder and Kart), then
-     *  freeing the kart will free this rewinder instance as well.
-     */
-    bool m_can_be_destroyed;
-
 public:
-             Rewinder(const std::string& ui, bool can_be_destroyed,
-                      bool auto_add = true);
-    virtual ~Rewinder();
+    Rewinder(const std::string& ui = "")             { m_unique_identity = ui; }
+
+    virtual ~Rewinder() {}
 
     /** Called before a rewind. Is used to save the previous position of an
      *  object before a rewind, so that the error due to a rewind can be
@@ -89,14 +82,17 @@ public:
     /** Nothing to do here. */
     virtual void reset() {}
     // -------------------------------------------------------------------------
-    /** True if this rewinder can be destroyed. Karts can not be destroyed,
-     *  cakes can. This is used by the RewindManager in reset. */
-    bool canBeDestroyed() const { return m_can_be_destroyed; }
-    // -------------------------------------------------------------------------
     virtual std::function<void()> getLocalStateRestoreFunction()
                                                              { return nullptr; }
     // -------------------------------------------------------------------------
-    const std::string& getUniqueIdentity() const   { return m_unique_identity; }
+    const std::string& getUniqueIdentity() const
+    {
+        assert(!m_unique_identity.empty() && m_unique_identity.size() < 255);
+        return m_unique_identity;
+    }
+    // -------------------------------------------------------------------------
+    bool add();
+
 };   // Rewinder
 #endif
 

src/physics/physical_object.cpp

@@ -136,7 +136,6 @@ PhysicalObject* PhysicalObject::fromXML(bool is_dynamic,
 PhysicalObject::PhysicalObject(bool is_dynamic,
                                const PhysicalObject::Settings& settings,
                                TrackObject* object)
-              : Rewinder("P", false/*can_be_destroyed*/, false/*auto_add*/)
 {
     m_shape              = NULL;
     m_body               = NULL;