Optimise PlayerRelationsManager::clear

Previous implementation was O(n^2), doing lots of work (saving file and
updating UI) for each removed player.

6 files changed, 34 insertions, 51 deletions

diff --git a/src/configuration.h b/src/configuration.h
index a22eb258..a26c6e20 100644
--- a/src/configuration.h
+++ b/src/configuration.h
@@ -53,7 +53,7 @@ class ConfigurationListManager
          * \return obj, or otherwise NULL to indicate that this option should
          *         be skipped
          */
-        virtual ConfigurationObject *writeConfigItem(T value,
+        virtual ConfigurationObject *writeConfigItem(const T &value,
                                                      ConfigurationObject *obj) = 0;
 
         /**
@@ -124,7 +124,7 @@ class ConfigurationObject
          */
         template <class IT, class T, class CONT>
         void setList(const std::string &name, IT begin, IT end,
-                     ConfigurationListManager<T, CONT> *manager)
+                     ConfigurationListManager<T, CONT> &manager)
         {
             auto *nextobj = new ConfigurationObject;
             std::list<ConfigurationObject *> &list = mContainerOptions[name];
@@ -132,7 +132,7 @@ class ConfigurationObject
 
             for (IT it = begin; it != end; it++)
             {
-                ConfigurationObject *wrobj = manager->writeConfigItem(*it, nextobj);
+                ConfigurationObject *wrobj = manager.writeConfigItem(*it, nextobj);
                 if (wrobj)
                 { // wrote something
                     assert (wrobj == nextobj);
@@ -160,12 +160,12 @@ class ConfigurationObject
         * \param manager An object capable of deserialising items into CONT
         */
         template<class T, class CONT>
-        CONT getList(const std::string &name, CONT empty, ConfigurationListManager<T, CONT> *manager)
+        CONT getList(const std::string &name, CONT empty, ConfigurationListManager<T, CONT> &manager)
         {
             CONT container = empty;
 
             for (auto obj : mContainerOptions[name])
-                container = manager->readConfigItem(obj, container);
+                container = manager.readConfigItem(obj, container);
 
             return container;
         }
diff --git a/src/gui/setup_players.cpp b/src/gui/setup_players.cpp
index ebe570bb..89c588c0 100644
--- a/src/gui/setup_players.cpp
+++ b/src/gui/setup_players.cpp
@@ -120,7 +120,7 @@ public:
             return RELATION_CHOICE_COLUMN_WIDTH;
     }
 
-    virtual void playerRelationsUpdated()
+    void playerRelationsUpdated()
     {
         signalBeforeUpdate();
 
@@ -303,18 +303,11 @@ void Setup_Players::reset()
     // We now have to search through the list of ignore choices to find the
     // current selection. We could use an index into the table of config
     // options in player_relations instead of strategies to sidestep this.
-    int selection = 0;
-    for (unsigned int i = 0;
-                      i < player_relations.getPlayerIgnoreStrategies().size();
-                      ++i)
-        if (player_relations.getPlayerIgnoreStrategies()[i] ==
-            player_relations.getPlayerIgnoreStrategy())
-        {
-
-            selection = i;
-            break;
-        }
+    const auto &strategies = player_relations.getPlayerIgnoreStrategies();
+    auto i = std::find(strategies.begin(), strategies.end(),
+                       player_relations.getPlayerIgnoreStrategy());
 
+    int selection = i == strategies.end() ? 0 : i - strategies.begin();
     mIgnoreActionChoicesBox->setSelected(selection);
 }
 
@@ -401,7 +394,7 @@ void Setup_Players::action(const gcn::ActionEvent &event)
     }
 }
 
-void Setup_Players::updatedPlayer(const std::string &name)
+void Setup_Players::playerRelationsUpdated()
 {
     mPlayerTableModel->playerRelationsUpdated();
     mDefaultTrading->setSelected(
diff --git a/src/gui/setup_players.h b/src/gui/setup_players.h
index 90663029..758c646d 100644
--- a/src/gui/setup_players.h
+++ b/src/gui/setup_players.h
@@ -48,7 +48,7 @@ public:
 
     void action(const gcn::ActionEvent &event) override;
 
-    void updatedPlayer(const std::string &name) override;
+    void playerRelationsUpdated() override;
 
 private:
     StaticTableModel *mPlayerTableTitleModel;
diff --git a/src/gui/widgets/tablemodel.h b/src/gui/widgets/tablemodel.h
index 2e36992a..d4274e39 100644
--- a/src/gui/widgets/tablemodel.h
+++ b/src/gui/widgets/tablemodel.h
@@ -77,20 +77,19 @@ public:
      */
     virtual gcn::Widget *getElementAt(int row, int column) const = 0;
 
-    virtual void installListener(TableModelListener *listener);
-
-    virtual void removeListener(TableModelListener *listener);
+    void installListener(TableModelListener *listener);
+    void removeListener(TableModelListener *listener);
 
 protected:
     /**
      * Tells all listeners that the table is about to see an update
      */
-    virtual void signalBeforeUpdate();
+    void signalBeforeUpdate();
 
     /**
      * Tells all listeners that the table has seen an update
      */
-    virtual void signalAfterUpdate();
+    void signalAfterUpdate();
 
 private:
     std::set<TableModelListener *> listeners;
diff --git a/src/playerrelations.cpp b/src/playerrelations.cpp
index 731092f3..fc9c991d 100644
--- a/src/playerrelations.cpp
+++ b/src/playerrelations.cpp
@@ -39,7 +39,7 @@
 class PlayerConfSerialiser : public ConfigurationListManager<std::pair<std::string, PlayerRelation>,
                                                              std::map<std::string, PlayerRelation> *>
 {
-    ConfigurationObject *writeConfigItem(std::pair<std::string, PlayerRelation> value,
+    ConfigurationObject *writeConfigItem(const std::pair<std::string, PlayerRelation> &value,
                                          ConfigurationObject *cobj) override
     {
         cobj->setValue(NAME, value.first);
@@ -68,8 +68,6 @@ class PlayerConfSerialiser : public ConfigurationListManager<std::pair<std::stri
     }
 };
 
-static PlayerConfSerialiser player_conf_serialiser; // stateless singleton
-
 const unsigned int PlayerRelation::RELATION_PERMISSIONS[RELATIONS_NR] = {
     /* NEUTRAL */    0, // we always fall back to the defaults anyway
     /* FRIEND  */    EMOTE | SPEECH_FLOAT | SPEECH_LOG | WHISPER | TRADE,
@@ -82,13 +80,6 @@ PlayerRelation::PlayerRelation(Relation relation)
     mRelation = relation;
 }
 
-PlayerRelationsManager::PlayerRelationsManager() :
-    mPersistIgnores(false),
-    mDefaultPermissions(PlayerRelation::DEFAULT),
-    mIgnoreStrategy(nullptr)
-{
-}
-
 PlayerRelationsManager::~PlayerRelationsManager()
 {
     delete_all(mIgnoreStrategies);
@@ -96,8 +87,8 @@ PlayerRelationsManager::~PlayerRelationsManager()
 
 void PlayerRelationsManager::clear()
 {
-    for (const auto &name : getPlayers())
-        removePlayer(name);
+    mRelations.clear();
+    signalUpdate();
 }
 
 #define PERSIST_IGNORE_LIST "persistent-player-list"
@@ -125,9 +116,10 @@ void PlayerRelationsManager::load()
     if (ignore_strategy_index >= 0)
         setPlayerIgnoreStrategy(getPlayerIgnoreStrategies()[ignore_strategy_index]);
 
+    PlayerConfSerialiser player_conf_serialiser;
     config.getList<std::pair<std::string, PlayerRelation>,
                    std::map<std::string, PlayerRelation> *>
-        ("player",  &mRelations, &player_conf_serialiser);
+        ("player",  &mRelations, player_conf_serialiser);
 }
 
 
@@ -141,12 +133,13 @@ void PlayerRelationsManager::init()
 
 void PlayerRelationsManager::store()
 {
+    PlayerConfSerialiser player_conf_serialiser;
     config.setList<std::map<std::string, PlayerRelation>::const_iterator,
                    std::pair<std::string, PlayerRelation>,
                    std::map<std::string, PlayerRelation> *>
         ("player",
          mRelations.begin(), mRelations.end(),
-         &player_conf_serialiser);
+         player_conf_serialiser);
 
     config.setValue(DEFAULT_PERMISSIONS, mDefaultPermissions);
     config.setValue(PERSIST_IGNORE_LIST, mPersistIgnores);
@@ -156,12 +149,12 @@ void PlayerRelationsManager::store()
     config.write();
 }
 
-void PlayerRelationsManager::signalUpdate(const std::string &name)
+void PlayerRelationsManager::signalUpdate()
 {
     store();
 
     for (auto listener : mListeners)
-        listener->updatedPlayer(name);
+        listener->playerRelationsUpdated();
 }
 
 unsigned int PlayerRelationsManager::checkPermissionSilently(
@@ -226,7 +219,7 @@ void PlayerRelationsManager::setRelation(const std::string &playerName,
                                          PlayerRelation::Relation relation)
 {
     mRelations[playerName] = PlayerRelation(relation);
-    signalUpdate(playerName);
+    signalUpdate();
 }
 
 std::vector<std::string> PlayerRelationsManager::getPlayers() const
@@ -247,7 +240,7 @@ void PlayerRelationsManager::removePlayer(const std::string &name)
     if (it != mRelations.end())
     {
         mRelations.erase(it);
-        signalUpdate(name);
+        signalUpdate();
     }
 }
 
@@ -272,9 +265,7 @@ unsigned int PlayerRelationsManager::getDefault() const
 void PlayerRelationsManager::setDefault(unsigned int permissions)
 {
     mDefaultPermissions = permissions;
-
-    store();
-    signalUpdate(std::string());
+    signalUpdate();
 }
 
 
diff --git a/src/playerrelations.h b/src/playerrelations.h
index 2a0426a1..e3183da6 100644
--- a/src/playerrelations.h
+++ b/src/playerrelations.h
@@ -85,7 +85,7 @@ public:
     PlayerRelationsListener() = default;
     virtual ~PlayerRelationsListener() = default;
 
-    virtual void updatedPlayer(const std::string &name) = 0;
+    virtual void playerRelationsUpdated() = 0;
 };
 
 /**
@@ -96,7 +96,7 @@ public:
 class PlayerRelationsManager
 {
 public:
-    PlayerRelationsManager();
+    PlayerRelationsManager() = default;
     ~PlayerRelationsManager();
 
     /**
@@ -222,12 +222,12 @@ public:
     }
 
 private:
-    void signalUpdate(const std::string &name);
+    void signalUpdate();
 
-    bool mPersistIgnores; // If NOT set, we delete the ignored data upon reloading
-    unsigned int mDefaultPermissions;
+    bool mPersistIgnores = false; // If NOT set, we delete the ignored data upon reloading
+    unsigned int mDefaultPermissions = PlayerRelation::DEFAULT;
 
-    PlayerIgnoreStrategy *mIgnoreStrategy;
+    PlayerIgnoreStrategy *mIgnoreStrategy = nullptr;
     std::map<std::string, PlayerRelation> mRelations;
     std::list<PlayerRelationsListener *> mListeners;
     std::vector<PlayerIgnoreStrategy *> mIgnoreStrategies;