Change CharacterManager to use list instead of vector

Solves a crash when deleting all savegames of a character due to mCurrent being invalidated

apps/openmw/mwbase/statemanager.hpp

@@ -1,7 +1,7 @@
 #ifndef GAME_MWSTATE_STATEMANAGER_H
 #define GAME_MWSTATE_STATEMANAGER_H
 
-#include <vector>
+#include <list>
 #include <string>
 
 namespace MWState
@@ -24,7 +24,7 @@ namespace MWBase
                 State_Running
             };
 
-            typedef std::vector<MWState::Character>::const_iterator CharacterIterator;
+            typedef std::list<MWState::Character>::const_iterator CharacterIterator;
 
         private:
 

apps/openmw/mwstate/charactermanager.cpp

@@ -49,20 +49,17 @@ MWState::Character *MWState::CharacterManager::getCurrentCharacter (bool create)
 
 void MWState::CharacterManager::deleteSlot(const MWState::Character *character, const MWState::Slot *slot)
 {
-    int index = character - &mCharacters[0];
+    std::list<Character>::iterator it = findCharacter(character);
 
-    if (index<0 || index>=static_cast<int> (mCharacters.size()))
-        throw std::logic_error ("invalid character");
-
-    mCharacters[index].deleteSlot(slot);
+    it->deleteSlot(slot);
 
-    if (mCharacters[index].begin() == mCharacters[index].end())
+    if (character->begin() == character->end())
     {
         // All slots deleted, cleanup and remove this character
-        mCharacters[index].cleanup();
+        it->cleanup();
         if (character == mCurrent)
             mCurrent = NULL;
-        mCharacters.erase(mCharacters.begin() + index);
+        mCharacters.erase(it);
     }
 }
 
@@ -78,14 +75,24 @@ void MWState::CharacterManager::createCharacter()
     mCurrent = &mCharacters.back();
 }
 
-void MWState::CharacterManager::setCurrentCharacter (const Character *character)
+std::list<MWState::Character>::iterator MWState::CharacterManager::findCharacter(const MWState::Character* character)
 {
-    int index = character - &mCharacters[0];
-
-    if (index<0 || index>=static_cast<int> (mCharacters.size()))
+    std::list<Character>::iterator it = mCharacters.begin();
+    for (; it != mCharacters.end(); ++it)
+    {
+        if (&*it == character)
+            break;
+    }
+    if (it == mCharacters.end())
         throw std::logic_error ("invalid character");
+    return it;
+}
+
+void MWState::CharacterManager::setCurrentCharacter (const Character *character)
+{
+    std::list<Character>::iterator it = findCharacter(character);
 
-    mCurrent = &mCharacters[index];
+    mCurrent = &*it;
 }
 
 void MWState::CharacterManager::clearCurrentCharacter()
@@ -93,12 +100,12 @@ void MWState::CharacterManager::clearCurrentCharacter()
     mCurrent = 0;
 }
 
-std::vector<MWState::Character>::const_iterator MWState::CharacterManager::begin() const
+std::list<MWState::Character>::const_iterator MWState::CharacterManager::begin() const
 {
     return mCharacters.begin();
 }
 
-std::vector<MWState::Character>::const_iterator MWState::CharacterManager::end() const
+std::list<MWState::Character>::const_iterator MWState::CharacterManager::end() const
 {
     return mCharacters.end();
 }

apps/openmw/mwstate/charactermanager.hpp

@@ -11,7 +11,10 @@ namespace MWState
     {
             boost::filesystem::path mPath;
             int mNext;
-            std::vector<Character> mCharacters;
+
+            // Uses std::list, so that mCurrent stays valid when characters are deleted
+            std::list<Character> mCharacters;
+
             Character *mCurrent;
             std::string mGame;
 
@@ -23,6 +26,8 @@ namespace MWState
             CharacterManager& operator= (const CharacterManager&);
             ///< Not implemented
 
+            std::list<Character>::iterator findCharacter(const MWState::Character* character);
+
         public:
 
             CharacterManager (const boost::filesystem::path& saves, const std::string& game);
@@ -39,9 +44,9 @@ namespace MWState
 
             void clearCurrentCharacter();
 
-            std::vector<Character>::const_iterator begin() const;
+            std::list<Character>::const_iterator begin() const;
 
-            std::vector<Character>::const_iterator end() const;
+            std::list<Character>::const_iterator end() const;
     };
 }