First pass at implementing state save/replay using an iterator-based approach.

Saving a state and later rewinding it now works, but still TODO is unwinding
(ie, replay in the future vs. in the past).

For now, debugging info remains, and some methods are commented out.  This
code is mostly for Thomas to inspect and make sure we're on the right track.

src/common/LinkedObjectPool.hxx

@@ -22,27 +22,30 @@
 #include "bspf.hxx"
 
 /**
- * A fixed-size object-pool based doubly-linked list that makes use of
+  A fixed-size object-pool based doubly-linked list that makes use of
- * multiple STL lists, to reduce frequent (de)allocations.
+  multiple STL lists, to reduce frequent (de)allocations.
- *
+
- * This structure can be used as either a stack or queue, but also allows
+  This structure can be used as either a stack or queue, but also allows
- * for removal at any location in the list.
+  for removal at any location in the list.
- *
+
- * There are two internal lists; one stores active nodes, and the other
+  There are two internal lists; one stores active nodes, and the other
- * stores pool nodes that have been 'deleted' from the active list (note
+  stores pool nodes that have been 'deleted' from the active list (note
- * that no actual deletion takes place; nodes are simply moved from one list
+  that no actual deletion takes place; nodes are simply moved from one list
- * to another).  Similarly, when a new node is added to the active list, it
+  to another).  Similarly, when a new node is added to the active list, it
- * is simply moved from the pool list to the active list.
+  is simply moved from the pool list to the active list.
- *
+
- * NOTE: For efficiency reasons, none of the methods check for overflow or
+  In all cases, the variable 'myCurrent' is updated to point to the
- *       underflow; that is the responsibility of the caller.
+  current node.
- *
+
- *       In the case of methods which wrap the C++ 'splice()' method, the
+  NOTE: You must always call 'currentIsValid()' before calling 'current()',
- *       semantics of splice are followed wrt invalid/out-of-range/etc
+        to make sure that the return value is a valid reference.
- *       iterators.  See the applicable documentation for such behaviour.
+
- *
+        In the case of methods which wrap the C++ 'splice()' method, the
- * @author Stephen Anthony
+        semantics of splice are followed wrt invalid/out-of-range/etc
- */
+        iterators.  See the applicable documentation for such behaviour.
+
+  @author Stephen Anthony
+*/
 namespace Common {
 
 template <class T, uInt32 CAPACITY = 100>
@@ -53,88 +56,140 @@ class LinkedObjectPool
     using const_iter = typename std::list<T>::const_iterator;
 
     /*
-     * Create a pool of size CAPACITY; the active list starts out empty.
+      Create a pool of size CAPACITY; the active list starts out empty.
-     */
+    */
-    LinkedObjectPool<T, CAPACITY>() {
+    LinkedObjectPool<T, CAPACITY>() : myCurrent(myList.end()) {
       for(uInt32 i = 0; i < CAPACITY; ++i)
         myPool.emplace_back(T());
     }
 
     /**
-     * Return a reference to the element at the first node in the active list.
+      Return node data that the 'current' iterator points to.
-     */
+      Note that this returns a valid value only in the case where the list
+      is non-empty (at least one node has been added to the active list).
+
+      Make sure to call 'currentIsValid()' before accessing this method.
+    */
+    T& current() { return *myCurrent; }
+
+    /**
+      Does the 'current' iterator point to a valid node in the active list?
+      This must be called before 'current()' is called.
+    */
+    bool currentIsValid() { return myCurrent != myList.end(); }
+
+    /**
+      Advance 'current' iterator to previous position in the active list.
+      If we go past the beginning, it is reset to one past the end (indicates nullptr).
+    */
+    void moveToPrevious() {
+      if(currentIsValid())
+        myCurrent = myCurrent == myList.begin() ? myList.end() : std::prev(myCurrent, 1);
+    }
+
+    /**
+      Advance 'current' iterator to next position in the active list.
+      If we go past the last node, it will point to one past the end (indicates nullptr).
+    */
+    void moveToNext() {
+      if(currentIsValid())
+        myCurrent = std::next(myCurrent, 1);
+    }
+
+#if 0
+    /**
+      Return a reference to the element at the first node in the active list.
+    */
     T& first() { return myList.front(); }
 
     /**
-     * Return a reference to the element at the last node in the active list.
+      Return a reference to the element at the last node in the active list.
-     */
+    */
     T& last() { return myList.back(); }
+#endif
 
     /**
-     * Add a new node at beginning of the active list, and return a reference
+      Add a new node at the beginning of the active list, and update 'current'
-     * to that nodes' data. The reference may then be modified; ie, you're
+      to point to that node.
-     * able to change the data located at that node.
+    */
-     */
+    void addFirst() {
-    T& addFirst() {
       myList.splice(myList.begin(), myPool, myPool.begin());
-      return myList.front();
+      myCurrent = myList.begin();
     }
 
     /**
-     * Add a new node at the end of the active list, and return a reference
+      Add a new node at the end of the active list, and update 'current'
-     * to that nodes' data. The reference may then be modified; ie, you're
+      to point to that node.
-     * able to change the data located at that node.
+    */
-     */
+    void addLast() {
-    T& addLast() {
       myList.splice(myList.end(), myPool, myPool.begin());
-      return myList.back();
+      myCurrent = std::prev(myList.end(), 1);
     }
 
     /**
-     * Remove the first node of the active list.
+      Remove the first node of the active list, updating 'current' if it
-     */
+      happens to be the one removed.
+    */
     void removeFirst() {
-      myPool.splice(myPool.end(), myList, myList.begin());
+      const_iter i = myList.begin();
+      myPool.splice(myPool.end(), myList, i);
+      if(myCurrent == i)  // did we just invalidate 'current'
+        moveToNext();     // if so, move to the next node
     }
 
     /**
-     * Remove the last node of the active list.
+      Remove the last node of the active list, updating 'current' if it
-     */
+      happens to be the one removed.
+    */
     void removeLast() {
-      myPool.splice(myPool.end(), myList, std::prev(myList.end(), 1));
+      const_iter i = std::prev(myList.end(), 1);
+      myPool.splice(myPool.end(), myList, i);
+      if(myCurrent == i)  // did we just invalidate 'current'
+        moveToPrevious(); // if so, move to the previous node
     }
 
+#if 0
     /**
-     * Convenience method to remove a single element from the active list at
+      Convenience method to remove a single element from the active list at
-     * position of the iterator +- the offset.
+      position of the iterator +- the offset.
-     */
+    */
     void remove(const_iter i, Int32 offset = 0) {
       myPool.splice(myPool.end(), myList,
                     offset >= 0 ? std::next(i, offset) : std::prev(i, -offset));
     }
 
     /**
-     * Convenience method to remove a single element from the active list by
+      Convenience method to remove a single element from the active list by
-     * index, offset from the beginning of the list. (ie, '0' means first
+      index, offset from the beginning of the list. (ie, '0' means first
-     * element, '1' is second, and so on).
+      element, '1' is second, and so on).
-     */
+    */
     void remove(uInt32 index) {
       myPool.splice(myPool.end(), myList, std::next(myList.begin(), index));
     }
+#endif
 
     /**
-     * Convenience method to remove a range of elements from 'index' to the
+      Remove range of elements from the beginning of the active list to
-     * end of the active list.
+      the 'current' node.
-     */
+    */
-    void removeLast(uInt32 index) {
+    void removeToFirst() {
-      myPool.splice(myPool.end(), myList, std::next(myList.begin(), index), myList.end());
+      myPool.splice(myPool.end(), myList, myList.begin(), myCurrent);
     }
 
     /**
-     * Erase entire contents of active list.
+      Remove range of elements from the node after 'current' to the end of the
-     */
+      active list.
+    */
+    void removeToLast() {
+      myPool.splice(myPool.end(), myList, std::next(myCurrent, 1), myList.end());
+    }
+
+    /**
+      Erase entire contents of active list.
+    */
     void clear() {
       myPool.splice(myPool.end(), myList, myList.begin(), myList.end());
+      myCurrent = myList.end();
     }
 
     /** Access the list with iterators, just as you would a normal C++ STL list */
@@ -150,6 +205,9 @@ class LinkedObjectPool
   private:
     std::list<T> myList, myPool;
 
+    // Current position in the active list (end() indicates an invalid position)
+    iter myCurrent;
+
   private:
     // Following constructors and assignment operators not supported
     LinkedObjectPool(const LinkedObjectPool&) = delete;

src/common/RewindManager.cxx

@@ -24,26 +24,29 @@
 
 #include "RewindManager.hxx"
 
+static int count = 1;
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 RewindManager::RewindManager(OSystem& system, StateManager& statemgr)
   : myOSystem(system),
     myStateManager(statemgr),
     myIsNTSC(true) // TODO
-    // TODO: current is not valid
 {
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool RewindManager::addState(const string& message)
 {
-  // TODO: remove following (preceding???) (all invalid) states
+  // Remove all future states
-  // myStateList.removeToFront(); from current() + 1 (or - 1???)
+  myStateList.removeToLast();
 
+  // Make sure we never run out of space
   if(myStateList.full())
     compressStates();
 
-  RewindState& state = myStateList.addFirst();
+  // Add new state at the end of the list (queue adds at end)
-  // TODO: addFirst() must set current() to the just added element
+  // This updates the 'current' iterator inside the list
+  myStateList.addLast();
+  RewindState& state = myStateList.current();
   Serializer& s = state.data;
 
   s.reset();  // rewind Serializer internal buffers
@@ -51,6 +54,8 @@ bool RewindManager::addState(const string& message)
   {
     state.message = message;
     state.cycle = myOSystem.console().tia().cycles();
+    state.count = count++;
+cerr << "add " << state.count << endl;
     return true;
   }
   return false;
@@ -59,14 +64,12 @@ bool RewindManager::addState(const string& message)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool RewindManager::rewindState()
 {
-  if(!myStateList.empty())
+  if(myStateList.currentIsValid())
   {
-    // TODO: get state previous to the current state instead of first()
+    RewindState& state = myStateList.current();
-    // RewindState& state = myStateList.current();
-    // myStateList.prev(); // moves current to the previous (older) element
-    RewindState& state = myStateList.first(); // TOOD: remove
     Serializer& s = state.data;
     string message = getMessage(state);
+cerr << "rewind " << state.count << endl;
 
     s.reset();  // rewind Serializer internal buffers
     myStateManager.loadState(s);
@@ -75,8 +78,9 @@ bool RewindManager::rewindState()
     // Show message indicating the rewind state
     myOSystem.frameBuffer().showMessage(message);
 
-    // TODO: Do NOT remove state (TODO later somewhere else: stop emulation)    
+    // Set internal current iterator to previous state (back in time),
-    myStateList.removeFirst(); // TODO: delete this
+    // since we've now processed this state
+    myStateList.moveToPrevious();
 
     return true;
   }
@@ -87,6 +91,7 @@ bool RewindManager::rewindState()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool RewindManager::unwindState()
 {
+#if 0
   if(!atFirst()) // or last???
   {
     // TODO: get state next to the current state
@@ -101,14 +106,15 @@ bool RewindManager::unwindState()
     // Show message indicating the rewind state
     myOSystem.frameBuffer().showMessage(message);*/
     return true;
-  }    
+  }
+#endif
   return false;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void RewindManager::compressStates()
 {
-  myStateList.removeLast();  // remove the oldest state file
+  myStateList.removeFirst();  // remove the oldest state file
   // TODO: add smart state removal
 }
 
@@ -122,7 +128,7 @@ string RewindManager::getMessage(RewindState& state)
     diffUnit;
   stringstream message;
   string unit;
-
+freq = NTSC_FREQ; // TODO: remove
   message << (diff >= 0 ? "Rewind" : "Unwind");
   diff = abs(diff);
 

src/common/RewindManager.hxx

@@ -25,7 +25,17 @@ class StateManager;
 #include "bspf.hxx"
 
 /**
-  This class is used to save (and later 'rewind') system save states.
+  This class is used to save (and later 'replay') system save states.
+  In this implementation, we assume states are added at the end of the list.
+
+  Rewinding involves moving the internal iterator backwards in time (towards
+  the beginning of the list).
+
+  Unwinding involves moving the internal iterator forwards in time (towards
+  the end of the list).
+
+  Any time a new state is added, the internal iterator moves back to the
+  insertion point of the data (the end of the list).
 
   @author  Stephen Anthony
 */
@@ -37,9 +47,9 @@ class RewindManager
   public:
     /**
       Add a new state file with the given message; this message will be
-      displayed when the state is rewound.
+      displayed when the state is replayed.
 
-      @param message  Message to display when rewinding to this state
+      @param message  Message to display when replaying this state
     */
     bool addState(const string& message);
 
@@ -70,12 +80,16 @@ class RewindManager
       Serializer data;
       string message;
       uInt64 cycle;
+      int count; //  TODO - remove this
 
       // We do nothing on object instantiation or copy
+      // The goal of LinkedObjectPool is to not do any allocations at all
       RewindState() { }
       RewindState(const RewindState&) { }
     };
 
+    // The linked-list to store states (internally it takes care of reducing
+    // frequent (de)-allocations)
     Common::LinkedObjectPool<RewindState, MAX_SIZE> myStateList;
 
     bool myIsNTSC;
@@ -84,7 +98,6 @@ class RewindManager
 
     string getMessage(RewindState& state);
 
-
   private:
     // Following constructors and assignment operators not supported
     RewindManager() = delete;