From 996e49c534f3f9fdc0efe860e8ebd74a51cb0458 Mon Sep 17 00:00:00 2001
From: scrawl <scrawl@baseoftrash.de>
Date: Mon, 2 Jun 2014 20:24:35 +0200
Subject: [PATCH] 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      |  4 +--
 apps/openmw/mwstate/charactermanager.cpp | 37 ++++++++++++++----------
 apps/openmw/mwstate/charactermanager.hpp | 11 +++++--
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/apps/openmw/mwbase/statemanager.hpp b/apps/openmw/mwbase/statemanager.hpp
index 121a73a48..006be921b 100644
--- a/apps/openmw/mwbase/statemanager.hpp
+++ b/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:
 
diff --git a/apps/openmw/mwstate/charactermanager.cpp b/apps/openmw/mwstate/charactermanager.cpp
index 822e2d88e..d773904db 100644
--- a/apps/openmw/mwstate/charactermanager.cpp
+++ b/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");
+    it->deleteSlot(slot);
 
-    mCharacters[index].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();
 }
 
+std::list<MWState::Character>::iterator MWState::CharacterManager::findCharacter(const MWState::Character* character)
+{
+    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)
 {
-    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");
-
-    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();
 }
diff --git a/apps/openmw/mwstate/charactermanager.hpp b/apps/openmw/mwstate/charactermanager.hpp
index 869d34f21..adf9d2ef4 100644
--- a/apps/openmw/mwstate/charactermanager.hpp
+++ b/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;
     };
 }