Turned the PlayerRelation struct into an enum class

Less code to achieve the same thing (strong type and namespaced values).

The permissions related values have been moved to a PlayerPermissions
struct, which is also a bit less confusing.

10 files changed, 82 insertions, 106 deletions

diff --git a/src/being.cpp b/src/being.cpp
index 7c77f19f..ef50d77e 100644
--- a/src/being.cpp
+++ b/src/being.cpp
@@ -1217,7 +1217,7 @@ void Being::event(Event::Channel channel, const Event &event)
     if (channel == Event::ChatChannel &&
             (event.getType() == Event::Being
              || event.getType() == Event::Player) &&
-            event.getInt("permissions") & PlayerRelation::SPEECH_FLOAT)
+            event.getInt("permissions") & PlayerPermissions::SPEECH_FLOAT)
     {
         try
         {
diff --git a/src/game.cpp b/src/game.cpp
index c0624425..96935981 100644
--- a/src/game.cpp
+++ b/src/game.cpp
@@ -703,15 +703,15 @@ void Game::handleInput()
                     case KeyboardConfig::KEY_TRADE:
                         // Toggle accepting of incoming trade requests
                         unsigned int deflt = player_relations.getDefault();
-                        if (deflt & PlayerRelation::TRADE)
+                        if (deflt & PlayerPermissions::TRADE)
                         {
                             serverNotice(_("Ignoring incoming trade requests"));
-                            deflt &= ~PlayerRelation::TRADE;
+                            deflt &= ~PlayerPermissions::TRADE;
                         }
                         else
                         {
                             serverNotice(_("Accepting incoming trade requests"));
-                            deflt |= PlayerRelation::TRADE;
+                            deflt |= PlayerPermissions::TRADE;
                         }
 
                         player_relations.setDefault(deflt);
diff --git a/src/gui/chatwindow.cpp b/src/gui/chatwindow.cpp
index 29bf9c1b..b4b03450 100644
--- a/src/gui/chatwindow.cpp
+++ b/src/gui/chatwindow.cpp
@@ -406,7 +406,7 @@ void ChatWindow::event(Event::Channel channel, const Event &event)
         }
         else if (event.getType() == Event::Being)
         {
-            if (event.getInt("permissions") & PlayerRelation::SPEECH_LOG)
+            if (event.getInt("permissions") & PlayerPermissions::SPEECH_LOG)
                 localChatTab->chatLog(event.getString("message"), BY_OTHER);
         }
     }
diff --git a/src/gui/setup_players.cpp b/src/gui/setup_players.cpp
index 89c588c0..a771d7ae 100644
--- a/src/gui/setup_players.cpp
+++ b/src/gui/setup_players.cpp
@@ -56,7 +56,7 @@ static const char *table_titles[COLUMNS_NR] =
     N_("Relation")
 };
 
-static const char *RELATION_NAMES[PlayerRelation::RELATIONS_NR] =
+static const char *RELATION_NAMES[RELATIONS_NR] =
 {
     N_("Neutral"),
     N_("Friend"),
@@ -71,7 +71,7 @@ public:
 
     int getNumberOfElements() override
     {
-        return PlayerRelation::RELATIONS_NR;
+        return RELATIONS_NR;
     }
 
     std::string getElementAt(int i) override
@@ -135,7 +135,7 @@ public:
             mWidgets.push_back(widget);
 
             gcn::DropDown *choicebox = new DropDown(mListModel);
-            choicebox->setSelected(player_relations.getRelation(name));
+            choicebox->setSelected(static_cast<int>(player_relations.getRelation(name)));
             mWidgets.push_back(choicebox);
         }
 
@@ -147,8 +147,8 @@ public:
         auto *choicebox = static_cast<gcn::DropDown *>(
                                    getElementAt(row, RELATION_CHOICE_COLUMN));
         player_relations.setRelation(getPlayerAt(row),
-                                   static_cast<PlayerRelation::Relation>(
-                                   choicebox->getSelected()));
+                                     static_cast<PlayerRelation>(
+                                         choicebox->getSelected()));
     }
 
 
@@ -212,9 +212,9 @@ Setup_Players::Setup_Players():
     mPlayerTitleTable(new GuiTable(mPlayerTableTitleModel)),
     mPlayerScrollArea(new ScrollArea(mPlayerTable)),
     mDefaultTrading(new CheckBox(_("Allow trading"),
-                player_relations.getDefault() & PlayerRelation::TRADE)),
+                player_relations.getDefault() & PlayerPermissions::TRADE)),
     mDefaultWhisper(new CheckBox(_("Allow whispers"),
-                player_relations.getDefault() & PlayerRelation::WHISPER)),
+                player_relations.getDefault() & PlayerPermissions::WHISPER)),
     mDeleteButton(new Button(_("Delete"), ACTION_DELETE, this)),
     mWhisperTab(config.getBoolValue("whispertab")),
     mWhisperTabCheckBox(new CheckBox(_("Put all whispers in tabs"), mWhisperTab)),
@@ -316,13 +316,13 @@ void Setup_Players::apply()
     player_relations.store();
 
     unsigned int old_default_relations = player_relations.getDefault() &
-                                         ~(PlayerRelation::TRADE |
-                                           PlayerRelation::WHISPER);
+                                         ~(PlayerPermissions::TRADE |
+                                           PlayerPermissions::WHISPER);
     player_relations.setDefault(old_default_relations
                                 | (mDefaultTrading->isSelected() ?
-                                       PlayerRelation::TRADE : 0)
+                                       PlayerPermissions::TRADE : 0)
                                 | (mDefaultWhisper->isSelected() ?
-                                       PlayerRelation::WHISPER : 0));
+                                       PlayerPermissions::WHISPER : 0));
     config.setValue("whispertab", mWhisperTab);
 
     bool showGender = config.getBoolValue("showgender");
@@ -398,7 +398,7 @@ void Setup_Players::playerRelationsUpdated()
 {
     mPlayerTableModel->playerRelationsUpdated();
     mDefaultTrading->setSelected(
-            player_relations.getDefault() & PlayerRelation::TRADE);
+            player_relations.getDefault() & PlayerPermissions::TRADE);
     mDefaultWhisper->setSelected(
-            player_relations.getDefault() & PlayerRelation::WHISPER);
+            player_relations.getDefault() & PlayerPermissions::WHISPER);
 }
diff --git a/src/net/manaserv/chathandler.cpp b/src/net/manaserv/chathandler.cpp
index 11a1331e..dca556c2 100644
--- a/src/net/manaserv/chathandler.cpp
+++ b/src/net/manaserv/chathandler.cpp
@@ -173,7 +173,7 @@ void ChatHandler::handleGameChatMessage(MessageIn &msg)
     event.setInt("beingId", id);
     event.setInt("permissions", player_relations
                  .checkPermissionSilently(being->getName(),
-                 PlayerRelation::SPEECH_LOG | PlayerRelation::SPEECH_FLOAT));
+                 PlayerPermissions::SPEECH_LOG | PlayerPermissions::SPEECH_FLOAT));
     event.trigger(Event::ChatChannel);
 }
 
diff --git a/src/net/tmwa/beinghandler.cpp b/src/net/tmwa/beinghandler.cpp
index 159352cc..e2c4f158 100644
--- a/src/net/tmwa/beinghandler.cpp
+++ b/src/net/tmwa/beinghandler.cpp
@@ -396,7 +396,7 @@ void BeingHandler::handleMessage(MessageIn &msg)
                 break;
             }
 
-            if (player_relations.hasPermission(dstBeing, PlayerRelation::EMOTE))
+            if (player_relations.hasPermission(dstBeing, PlayerPermissions::EMOTE))
             {
                 const int fx = EmoteDB::get(msg.readInt8() - 1).effectId;
                 effectManager->trigger(fx, dstBeing);
diff --git a/src/net/tmwa/chathandler.cpp b/src/net/tmwa/chathandler.cpp
index 883ae083..4f998922 100644
--- a/src/net/tmwa/chathandler.cpp
+++ b/src/net/tmwa/chathandler.cpp
@@ -129,7 +129,7 @@ void ChatHandler::handleMessage(MessageIn &msg)
                         chatMsg.find("!sellitem ") == 0)
                     return;
 
-                if (player_relations.hasPermission(nick, PlayerRelation::WHISPER))
+                if (player_relations.hasPermission(nick, PlayerPermissions::WHISPER))
                 {
                     Event event(Event::Whisper);
                     event.setString("nick", nick);
@@ -176,13 +176,13 @@ void ChatHandler::handleMessage(MessageIn &msg)
             if (being->getType() == Being::PLAYER)
             {
                 perms = player_relations.checkPermissionSilently(sender_name,
-                    PlayerRelation::SPEECH_LOG | PlayerRelation::SPEECH_FLOAT);
+                    PlayerPermissions::SPEECH_LOG | PlayerPermissions::SPEECH_FLOAT);
             }
             else
             {
                 perms = player_relations.getDefault()
-                        & (PlayerRelation::SPEECH_LOG
-                           | PlayerRelation::SPEECH_FLOAT);
+                        & (PlayerPermissions::SPEECH_LOG
+                           | PlayerPermissions::SPEECH_FLOAT);
             }
 
             // This is a sure sign of some special command of manaplus,
@@ -232,8 +232,8 @@ void ChatHandler::handleMessage(MessageIn &msg)
                 event.setString("nick", local_player->getName());
                 event.setInt("beingId", local_player->getId());
                 event.setInt("permissions", player_relations.getDefault()
-                             & (PlayerRelation::SPEECH_LOG
-                                | PlayerRelation::SPEECH_FLOAT));
+                             & (PlayerPermissions::SPEECH_LOG
+                                | PlayerPermissions::SPEECH_FLOAT));
                 event.trigger(Event::ChatChannel);
             }
             else
diff --git a/src/net/tmwa/tradehandler.cpp b/src/net/tmwa/tradehandler.cpp
index da82d751..60732eef 100644
--- a/src/net/tmwa/tradehandler.cpp
+++ b/src/net/tmwa/tradehandler.cpp
@@ -96,7 +96,7 @@ void TradeHandler::handleMessage(MessageIn &msg)
                 std::string tradePartnerNameTemp = msg.readString(24);
 
                 if (player_relations.hasPermission(tradePartnerName,
-                                                   PlayerRelation::TRADE))
+                                                   PlayerPermissions::TRADE))
                 {
                     if (mTrading || confirmDlg)
                     {
@@ -142,7 +142,7 @@ void TradeHandler::handleMessage(MessageIn &msg)
                     break;
                 case 4: // Trade cancelled
                     if (player_relations.hasPermission(tradePartnerName,
-                                                       PlayerRelation::SPEECH_LOG))
+                                                       PlayerPermissions::SPEECH_LOG))
                     {
                         serverNotice(strprintf(_("Trade with %s cancelled."),
                                                tradePartnerName.c_str()));
diff --git a/src/playerrelations.cpp b/src/playerrelations.cpp
index fc9c991d..f78852e8 100644
--- a/src/playerrelations.cpp
+++ b/src/playerrelations.cpp
@@ -1,7 +1,7 @@
 /*
  *  The Mana Client
  *  Copyright (C) 2008-2009  The Mana World Development Team
- *  Copyright (C) 2009-2012  The Mana Developers
+ *  Copyright (C) 2009-2024  The Mana Developers
  *
  *  This file is part of The Mana Client.
  *
@@ -43,7 +43,7 @@ class PlayerConfSerialiser : public ConfigurationListManager<std::pair<std::stri
                                          ConfigurationObject *cobj) override
     {
         cobj->setValue(NAME, value.first);
-        cobj->setValue(RELATION, toString(value.second.mRelation));
+        cobj->setValue(RELATION, toString(static_cast<int>(value.second)));
 
         return cobj;
     }
@@ -52,15 +52,15 @@ class PlayerConfSerialiser : public ConfigurationListManager<std::pair<std::stri
     readConfigItem(ConfigurationObject *cobj,
                    std::map<std::string, PlayerRelation> *container) override
     {
-        std::string name = cobj->getValue(NAME, "");
+        std::string name = cobj->getValue(NAME, std::string());
         if (name.empty())
             return container;
 
         auto it = (*container).find(name);
         if (it != (*container).end())
         {
-            int v = (int)cobj->getValue(RELATION, PlayerRelation::NEUTRAL);
-            (*container)[name] = PlayerRelation(static_cast<PlayerRelation::Relation>(v));
+            int v = cobj->getValue(RELATION, static_cast<int>(PlayerRelation::NEUTRAL));
+            (*container)[name] = static_cast<PlayerRelation>(v);
         }
         // otherwise ignore the duplicate entry
 
@@ -68,17 +68,7 @@ class PlayerConfSerialiser : public ConfigurationListManager<std::pair<std::stri
     }
 };
 
-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,
-    /* DISREGARDED*/ EMOTE | SPEECH_FLOAT,
-    /* IGNORED */    0
-};
 
-PlayerRelation::PlayerRelation(Relation relation)
-{
-    mRelation = relation;
-}
 
 PlayerRelationsManager::~PlayerRelationsManager()
 {
@@ -161,27 +151,32 @@ unsigned int PlayerRelationsManager::checkPermissionSilently(
                                                   const std::string &playerName,
                                                   unsigned int flags)
 {
-    auto it = mRelations.find(playerName);
-    if (it == mRelations.end())
-        return mDefaultPermissions & flags;
+    unsigned int permissions = mDefaultPermissions;
 
-    PlayerRelation &r = it->second;
-
-    unsigned int permissions =
-        PlayerRelation::RELATION_PERMISSIONS[r.mRelation];
-
-    switch (r.mRelation)
+    switch (getRelation(playerName))
     {
     case PlayerRelation::NEUTRAL:
-        permissions = mDefaultPermissions;
         break;
 
+        // widen permissions for friends
     case PlayerRelation::FRIEND:
-        permissions |= mDefaultPermissions; // widen
+        permissions |=
+                PlayerPermissions::EMOTE |
+                PlayerPermissions::SPEECH_FLOAT |
+                PlayerPermissions::SPEECH_LOG |
+                PlayerPermissions::WHISPER |
+                PlayerPermissions::TRADE;
         break;
 
-    default:
-        permissions &= mDefaultPermissions; // narrow
+        // narrow permissions for disregarded and ignored players
+    case PlayerRelation::DISREGARDED:
+        permissions &=
+                PlayerPermissions::EMOTE |
+                PlayerPermissions::SPEECH_FLOAT;
+        break;
+    case PlayerRelation::IGNORED:
+        permissions &= 0;
+        break;
     }
 
     return permissions & flags;
@@ -200,25 +195,20 @@ bool PlayerRelationsManager::hasPermission(const std::string &name,
     unsigned int rejections = flags & ~checkPermissionSilently(name, flags);
     bool permitted = rejections == 0;
 
-    if (!permitted)
+    // execute `ignore' strategy, if possible
+    if (!permitted && mIgnoreStrategy)
     {
-        // execute `ignore' strategy, if possible
-        if (mIgnoreStrategy)
-        {
-            Being *b = actorSpriteManager->findBeingByName(name,
-                                                        ActorSprite::PLAYER);
-            if (b && b->getType() == ActorSprite::PLAYER)
-                mIgnoreStrategy->ignore(b, rejections);
-        }
+        if (Being *b = actorSpriteManager->findBeingByName(name, ActorSprite::PLAYER))
+            mIgnoreStrategy->ignore(b, rejections);
     }
 
     return permitted;
 }
 
 void PlayerRelationsManager::setRelation(const std::string &playerName,
-                                         PlayerRelation::Relation relation)
+                                         PlayerRelation relation)
 {
-    mRelations[playerName] = PlayerRelation(relation);
+    mRelations[playerName] = relation;
     signalUpdate();
 }
 
@@ -245,13 +235,10 @@ void PlayerRelationsManager::removePlayer(const std::string &name)
 }
 
 
-PlayerRelation::Relation PlayerRelationsManager::getRelation(const std::string &name) const
+PlayerRelation PlayerRelationsManager::getRelation(const std::string &name) const
 {
     auto it = mRelations.find(name);
-    if (it != mRelations.end())
-        return it->second.mRelation;
-
-    return PlayerRelation::NEUTRAL;
+    return it != mRelations.end() ? it->second : PlayerRelation::NEUTRAL;
 }
 
 ////////////////////////////////////////
diff --git a/src/playerrelations.h b/src/playerrelations.h
index e3183da6..45102fd9 100644
--- a/src/playerrelations.h
+++ b/src/playerrelations.h
@@ -1,7 +1,7 @@
 /*
  *  The Mana Client
  *  Copyright (C) 2008-2009  The Mana World Development Team
- *  Copyright (C) 2009-2012  The Mana Developers
+ *  Copyright (C) 2009-2024  The Mana Developers
  *
  *  This file is part of The Mana Client.
  *
@@ -29,38 +29,28 @@
 
 class Being;
 
-struct PlayerRelation
+enum class PlayerRelation
 {
-    static const unsigned int EMOTE        = (1 << 0);
-    static const unsigned int SPEECH_FLOAT = (1 << 1);
-    static const unsigned int SPEECH_LOG   = (1 << 2);
-    static const unsigned int WHISPER      = (1 << 3);
-    static const unsigned int TRADE        = (1 << 4);
-
-    /**
-     * There are four types of relations:
-     * NEUTRAL, FRIEND, DISREGARDED, and IGNORED.
-     */
-    static const unsigned int RELATIONS_NR = 4;
-    static const unsigned int RELATION_PERMISSIONS[RELATIONS_NR];
-
-    static const unsigned int DEFAULT = EMOTE
-                                      | SPEECH_FLOAT
-                                      | SPEECH_LOG
-                                      | WHISPER
-                                      | TRADE;
-    enum Relation {
-        NEUTRAL     = 0,
-        FRIEND      = 1,
-        DISREGARDED = 2,
-        IGNORED     = 3
-    };
-
-    explicit PlayerRelation(Relation relation = NEUTRAL);
-
-    Relation mRelation; // bitmask for all of the above
+    NEUTRAL     = 0,
+    FRIEND      = 1,
+    DISREGARDED = 2,
+    IGNORED     = 3
 };
+constexpr unsigned int RELATIONS_NR = 4;
 
+struct PlayerPermissions
+{
+    // Used as flags
+    enum {
+        EMOTE           = 1 << 0,
+        SPEECH_FLOAT    = 1 << 1,
+        SPEECH_LOG      = 1 << 2,
+        WHISPER         = 1 << 3,
+        TRADE           = 1 << 4,
+
+        DEFAULT         = EMOTE | SPEECH_FLOAT | SPEECH_LOG | WHISPER | TRADE
+    };
+};
 
 /**
  * Ignore strategy: describes how we should handle ignores.
@@ -133,13 +123,12 @@ public:
     /**
      * Updates the relationship with this player.
      */
-    void setRelation(const std::string &name,
-                     PlayerRelation::Relation relation);
+    void setRelation(const std::string &name, PlayerRelation relation);
 
     /**
      * Updates the relationship with this player.
      */
-    PlayerRelation::Relation getRelation(const std::string &name) const;
+    PlayerRelation getRelation(const std::string &name) const;
 
     /**
      * Deletes the information recorded for a player.
@@ -225,7 +214,7 @@ private:
     void signalUpdate();
 
     bool mPersistIgnores = false; // If NOT set, we delete the ignored data upon reloading
-    unsigned int mDefaultPermissions = PlayerRelation::DEFAULT;
+    unsigned int mDefaultPermissions = PlayerPermissions::DEFAULT;
 
     PlayerIgnoreStrategy *mIgnoreStrategy = nullptr;
     std::map<std::string, PlayerRelation> mRelations;