Improve playerrelation class.

3 files changed, 62 insertions, 46 deletions

diff --git a/src/configuration.h b/src/configuration.h
index 1ac38e9aa..1d703fd99 100644
--- a/src/configuration.h
+++ b/src/configuration.h
@@ -56,8 +56,9 @@ class ConfigurationListManager
          * \return obj, or otherwise NULL to indicate that this option should
          *         be skipped
          */
-        virtual ConfigurationObject
-            *writeConfigItem(T value, ConfigurationObject *obj) = 0;
+        virtual ConfigurationObject *writeConfigItem(const T &value,
+                                                     ConfigurationObject
+                                                     *const obj) const = 0;
 
         /**
          * Reads a value from a configuration object
@@ -65,8 +66,9 @@ class ConfigurationListManager
          * \param obj The configuration object to read from
          * \param container The container to insert the object to
          */
-        virtual CONT readConfigItem(ConfigurationObject *const obj,
-                                    CONT container) A_WARN_UNUSED = 0;
+        virtual CONT readConfigItem(const ConfigurationObject *const obj,
+                                    CONT container)
+                                    const A_WARN_UNUSED = 0;
 
         virtual ~ConfigurationListManager() {}
 };
diff --git a/src/playerrelations.cpp b/src/playerrelations.cpp
index cfba142ed..1e68eef5d 100644
--- a/src/playerrelations.cpp
+++ b/src/playerrelations.cpp
@@ -72,8 +72,8 @@ class PlayerConfSerialiser final :
 {
 public:
     virtual ConfigurationObject *writeConfigItem(
-        std::pair<std::string, PlayerRelation *> value,
-        ConfigurationObject *cobj) override
+        const std::pair<std::string, PlayerRelation *> &value,
+        ConfigurationObject *const cobj) const override
     {
         if (!cobj || !value.second)
             return nullptr;
@@ -85,12 +85,13 @@ public:
     }
 
     virtual std::map<std::string, PlayerRelation *> *
-    readConfigItem(ConfigurationObject *const cobj,
-                   std::map<std::string, PlayerRelation *> *container) override
+    readConfigItem(const ConfigurationObject *const cobj,
+                   std::map<std::string, PlayerRelation *>
+                   *const container) const override
     {
         if (!cobj)
             return container;
-        std::string name = cobj->getValue(NAME, "");
+        const std::string name = cobj->getValue(NAME, "");
         if (name.empty())
             return container;
 
@@ -144,7 +145,7 @@ PlayerRelationsManager::~PlayerRelationsManager()
 
 void PlayerRelationsManager::clear()
 {
-    StringVect *names = getPlayers();
+    StringVect *const names = getPlayers();
     FOR_EACHP (StringVectCIter, it, names)
         removePlayer(*it);
     delete names;
@@ -157,7 +158,7 @@ static const char *const DEFAULT_PERMISSIONS = "default-player-permissions";
 int PlayerRelationsManager::getPlayerIgnoreStrategyIndex(
     const std::string &name)
 {
-    std::vector<PlayerIgnoreStrategy *> *const strategies
+    const std::vector<PlayerIgnoreStrategy *> *const strategies
         = getPlayerIgnoreStrategies();
 
     if (!strategies)
@@ -185,8 +186,8 @@ void PlayerRelationsManager::load(const bool oldConfig)
     mDefaultPermissions = static_cast<int>(cfg->getValue(DEFAULT_PERMISSIONS,
                                            mDefaultPermissions));
 
-    std::string ignore_strategy_name = cfg->getValue(PLAYER_IGNORE_STRATEGY,
-                                                     DEFAULT_IGNORE_STRATEGY);
+    const std::string ignore_strategy_name = cfg->getValue(
+        PLAYER_IGNORE_STRATEGY, DEFAULT_IGNORE_STRATEGY);
     const int ignore_strategy_index = getPlayerIgnoreStrategyIndex(
         ignore_strategy_name);
 
@@ -216,15 +217,14 @@ void PlayerRelationsManager::init()
         (*it)->updateAll();
 }
 
-void PlayerRelationsManager::store()
+void PlayerRelationsManager::store() const
 {
     serverConfig.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", mRelations.begin(), mRelations.end(),
+        &player_conf_serialiser);
 
     serverConfig.setValue(DEFAULT_PERMISSIONS, mDefaultPermissions);
     serverConfig.setValue(PERSIST_IGNORE_LIST, mPersistIgnores);
@@ -251,15 +251,17 @@ void PlayerRelationsManager::signalUpdate(const std::string &name)
 }
 
 unsigned int PlayerRelationsManager::checkPermissionSilently(
-    const std::string &player_name, const unsigned int flags)
+    const std::string &player_name, const unsigned int flags) const
 {
-    const PlayerRelation *const r = mRelations[player_name];
-    if (!r)
+    const std::map<std::string, PlayerRelation *>::const_iterator
+        it = mRelations.find(player_name);
+    if (it == mRelations.end())
     {
         return mDefaultPermissions & flags;
     }
     else
     {
+        const PlayerRelation *const r = (*it).second;
         unsigned int permissions =
             PlayerRelation::RELATION_PERMISSIONS[r->mRelation];
 
@@ -287,7 +289,7 @@ unsigned int PlayerRelationsManager::checkPermissionSilently(
 }
 
 bool PlayerRelationsManager::hasPermission(const Being *const being,
-                                           const unsigned int flags)
+                                           const unsigned int flags) const
 {
     if (!being)
         return false;
@@ -298,7 +300,7 @@ bool PlayerRelationsManager::hasPermission(const Being *const being,
 }
 
 bool PlayerRelationsManager::hasPermission(const std::string &name,
-                                           const unsigned int flags)
+                                           const unsigned int flags) const
 {
     if (!actorSpriteManager)
         return false;
@@ -342,7 +344,7 @@ void PlayerRelationsManager::setRelation(const std::string &player_name,
     signalUpdate(player_name);
 }
 
-StringVect * PlayerRelationsManager::getPlayers()
+StringVect *PlayerRelationsManager::getPlayers() const
 {
     StringVect *const retval = new StringVect();
 
@@ -358,7 +360,7 @@ StringVect * PlayerRelationsManager::getPlayers()
 }
 
 StringVect *PlayerRelationsManager::getPlayersByRelation(
-    const PlayerRelation::Relation rel)
+    const PlayerRelation::Relation rel) const
 {
     StringVect *const retval = new StringVect();
 
@@ -385,10 +387,12 @@ void PlayerRelationsManager::removePlayer(const std::string &name)
 
 
 PlayerRelation::Relation PlayerRelationsManager::getRelation(
-    const std::string &name)
+    const std::string &name) const
 {
-    if (mRelations[name])
-        return mRelations[name]->mRelation;
+    const std::map<std::string, PlayerRelation *>::const_iterator
+        it = mRelations.find(name);
+    if (it != mRelations.end())
+        return (*it).second->mRelation;
 
     return PlayerRelation::NEUTRAL;
 }
@@ -409,7 +413,7 @@ void PlayerRelationsManager::setDefault(const unsigned int permissions)
     signalUpdate("");
 }
 
-void PlayerRelationsManager::ignoreTrade(std::string name)
+void PlayerRelationsManager::ignoreTrade(const std::string &name)
 {
     if (name.empty())
         return;
@@ -429,7 +433,7 @@ void PlayerRelationsManager::ignoreTrade(std::string name)
     }
 }
 
-bool PlayerRelationsManager::checkBadRelation(std::string name)
+bool PlayerRelationsManager::checkBadRelation(const std::string &name) const
 {
     if (name.empty())
         return true;
@@ -552,17 +556,22 @@ PlayerRelationsManager::getPlayerIgnoreStrategies()
     return &mIgnoreStrategies;
 }
 
-bool PlayerRelationsManager::isGoodName(std::string name)
+bool PlayerRelationsManager::isGoodName(const std::string &name) const
 {
     const size_t size = name.size();
 
-    if (size < 3 || mRelations[name])
+    if (size < 3)
+        return true;
+
+    const std::map<std::string, PlayerRelation *>::const_iterator
+        it = mRelations.find(name);
+    if (it != mRelations.end())
         return true;
 
     return checkName(name);
 }
 
-bool PlayerRelationsManager::isGoodName(Being *const being)
+bool PlayerRelationsManager::isGoodName(Being *const being) const
 {
     if (!being)
         return false;
@@ -572,7 +581,12 @@ bool PlayerRelationsManager::isGoodName(Being *const being)
     const std::string name = being->getName();
     const size_t size = name.size();
 
-    if (size < 3 || mRelations[name])
+    if (size < 3)
+        return true;
+
+    const std::map<std::string, PlayerRelation *>::const_iterator
+        it = mRelations.find(name);
+    if (it != mRelations.end())
         return true;
 
     const bool status = checkName(name);
@@ -583,9 +597,9 @@ bool PlayerRelationsManager::isGoodName(Being *const being)
 bool PlayerRelationsManager::checkName(const std::string &name) const
 {
     const size_t size = name.size();
-    std::string check = config.getStringValue("unsecureChars");
+    const std::string check = config.getStringValue("unsecureChars");
+    const std::string lastChar = name.substr(size - 1, 1);
 
-    std::string lastChar = name.substr(size - 1, 1);
     if (name.substr(0, 1) == " " || lastChar == " " || lastChar == "."
         || name.find("  ") != std::string::npos)
     {
diff --git a/src/playerrelations.h b/src/playerrelations.h
index d301a3f3b..fb185ce60 100644
--- a/src/playerrelations.h
+++ b/src/playerrelations.h
@@ -129,7 +129,7 @@ class PlayerRelationsManager final
         /**
          * Save configuration to our config file.
          */
-        void store();
+        void store() const;
 
         /**
          * Determines whether the player in question is being ignored, filtered by
@@ -137,7 +137,7 @@ class PlayerRelationsManager final
          */
         unsigned int checkPermissionSilently(const std::string &player_name,
                                              const unsigned int flags)
-                                             A_WARN_UNUSED;
+                                             const A_WARN_UNUSED;
 
         /**
          * Tests whether the player in question is being ignored for any of the
@@ -145,10 +145,10 @@ class PlayerRelationsManager final
          * if requested by the player.
          */
         bool hasPermission(const Being *const being,
-                           const unsigned int flags) A_WARN_UNUSED;
+                           const unsigned int flags) const A_WARN_UNUSED;
 
         bool hasPermission(const std::string &being,
-                           const unsigned int flags) A_WARN_UNUSED;
+                           const unsigned int flags) const A_WARN_UNUSED;
 
         /**
          * Updates the relationship with this player.
@@ -160,7 +160,7 @@ class PlayerRelationsManager final
          * Updates the relationship with this player.
          */
         PlayerRelation::Relation getRelation(const std::string &name)
-                                             A_WARN_UNUSED;
+                                             const A_WARN_UNUSED;
 
         /**
          * Deletes the information recorded for a player.
@@ -214,10 +214,10 @@ class PlayerRelationsManager final
          * Retrieves a sorted vector of all players for which we have any
          * relations recorded.
          */
-        StringVect *getPlayers() A_WARN_UNUSED;
+        StringVect *getPlayers() const A_WARN_UNUSED;
 
         StringVect *getPlayersByRelation(const PlayerRelation::Relation rel)
-                                         A_WARN_UNUSED;
+                                         const A_WARN_UNUSED;
 
         /**
          * Removes all recorded player info.
@@ -230,11 +230,11 @@ class PlayerRelationsManager final
         bool getPersistIgnores() const
         { return mPersistIgnores; }
 
-        void ignoreTrade(std::string name);
+        void ignoreTrade(const std::string &name);
 
-        bool isGoodName(Being *const being) A_WARN_UNUSED;
+        bool isGoodName(Being *const being) const A_WARN_UNUSED;
 
-        bool isGoodName(std::string name) A_WARN_UNUSED;
+        bool isGoodName(const std::string &name) const A_WARN_UNUSED;
 
         /**
          * Change the `ignore persist' flag.
@@ -250,7 +250,7 @@ class PlayerRelationsManager final
         void removeListener(PlayerRelationsListener *const listener)
         { mListeners.remove(listener); }
 
-        bool checkBadRelation(std::string name) A_WARN_UNUSED;
+        bool checkBadRelation(const std::string &name) const A_WARN_UNUSED;
 
     private:
         void signalUpdate(const std::string &name);