Merge pull request #221 from Salanto/change-userID-shenanigans

Simplify code for user id assignment

core/include/aoclient.h

@@ -2305,6 +2305,11 @@ class AOClient : public QObject {
      *        when modcall logging is used.
      */
     void logModcall(const QString& f_charName, const QString &f_ipid, const QString& f_oocName, const QString& f_areaName);
+
+    /**
+     * @brief Signals the server that the client has disconnected and marks its userID as free again.
+     */
+    void clientSuccessfullyDisconnected(const int& f_user_id);
 };
 
 #endif // AOCLIENT_H

core/include/server.h

@@ -184,6 +184,17 @@ class Server : public QObject {
      */
     QVector<AOClient*> m_clients;
 
+    /**
+     * @brief Collection of all clients with their userID as key.
+     */
+    QHash<int,AOClient*> m_clients_ids;
+
+    /**
+     * @brief Queue of all available IDs for clients. When this is empty the server
+     * rejects any new connection attempt.
+     */
+    QQueue<int> m_available_ids;
+
     /**
      * @brief The overall player count in the server.
      */
@@ -282,6 +293,11 @@ class Server : public QObject {
      */
     void handleDiscordIntegration();
 
+    /**
+     * @brief Marks a userID as free and ads it back to the available client id queue.
+     */
+    void markIDFree(const int& f_user_id);
+
   signals:
 
     /**
@@ -333,9 +349,9 @@ class Server : public QObject {
 
   private:
     /**
-     * @brief Connects new AOClient to the logger.
+     * @brief Connects new AOClient to logger and disconnect handling.
      **/
-    void hookupLogger(AOClient* client);
+    void hookupAOClient(AOClient* client);
 
     /**
      * @brief The proxy used for WebSocket connections.

core/src/aoclient.cpp

@@ -72,6 +72,8 @@ void AOClient::clientDisconnected()
     if (l_updateLocks)
         arup(ARUPType::LOCKED, true);
     arup(ARUPType::CM, true);
+
+    emit clientSuccessfullyDisconnected(m_id);
 }
 
 void AOClient::handlePacket(AOPacket packet)

core/src/server.cpp

@@ -105,24 +105,33 @@ void Server::start()
     m_ipban_list = ConfigManager::iprangeBans();
 
     //Rate-Limiter for IC-Chat
-    connect(&next_message_timer, SIGNAL(timeout()), this, SLOT(allowMessage()));
+    connect(&next_message_timer, &QTimer::timeout, this, &Server::allowMessage);
+
+    //Prepare player IDs and reference hash.
+    for (int i = 0; i <= ConfigManager::maxPlayers() - 1; i++){
+        m_available_ids.enqueue(i);
+        m_clients_ids.insert(i, nullptr);
+    }
 }
 
 void Server::clientConnected()
 {
     QTcpSocket* socket = server->nextPendingConnection();
-    int user_id;
-    QList<int> user_ids;
-    for (AOClient* client : qAsConst(m_clients)) {
-        user_ids.append(client->m_id);
-    }
-    for (user_id = 0; user_id <= m_player_count; user_id++) {
-        if (user_ids.contains(user_id))
-            continue;
-        else
-            break;
+
+    //Too many players. Reject connection!
+    //This also enforces the maximum playercount.
+    if (m_available_ids.empty()) {
+        AOPacket disconnect_reason("BD", {"Maximum playercount has been reached."});
+        socket->write(disconnect_reason.toUtf8());
+        socket->flush();
+        socket->close();
+        socket->deleteLater();
+        return;
     }
+
+    int user_id = m_available_ids.dequeue();
     AOClient* client = new AOClient(this, socket, this, user_id);
+    m_clients_ids.insert(user_id, client);
 
     int multiclient_count = 1;
     bool is_at_multiclient_limit = false;
@@ -134,7 +143,7 @@ void Server::clientConnected()
             multiclient_count++;
     }
 
-    if (multiclient_count > ConfigManager::multiClientLimit() && !client->m_remote_ip.isLoopback()) // TODO: make this configurable
+    if (multiclient_count > ConfigManager::multiClientLimit() && !client->m_remote_ip.isLoopback())
         is_at_multiclient_limit = true;
 
     if (is_banned) {
@@ -146,6 +155,7 @@ void Server::clientConnected()
         socket->flush();
         client->deleteLater();
         socket->close();
+        markIDFree(user_id);
         return;
     }
 
@@ -160,6 +170,7 @@ void Server::clientConnected()
         socket->write(l_ban_reason.toUtf8());
         client->deleteLater();
         socket->close();
+        markIDFree(user_id);
         return;
     }
 
@@ -176,7 +187,7 @@ void Server::clientConnected()
                                                     // tsuserver4. It should disable fantacrypt
                                                     // completely in any client 2.4.3 or newer
     client->sendPacket(decryptor);
-    hookupLogger(client);
+    hookupAOClient(client);
 #ifdef NET_DEBUG
     qDebug() << client->remote_ip.toString() << "connected";
 #endif
@@ -305,11 +316,7 @@ QList<AOClient*> Server::getClientsByIpid(QString ipid)
 
 AOClient* Server::getClientByID(int id)
 {
-    for (AOClient* client : qAsConst(m_clients)) {
-        if (client->m_id == id)
-            return client;
-    }
-    return nullptr;
+    return m_clients_ids.value(id);
 }
 
 int Server::getCharID(QString char_name)
@@ -354,7 +361,13 @@ void Server::handleDiscordIntegration()
     return;
 }
 
-void Server::hookupLogger(AOClient* client)
+void Server::markIDFree(const int &f_user_id)
+{
+    m_available_ids.enqueue(f_user_id);
+    m_clients_ids.insert(f_user_id, nullptr);
+}
+
+void Server::hookupAOClient(AOClient* client)
 {
     connect(client, &AOClient::logIC,
             logger, &ULogger::logIC);
@@ -370,6 +383,8 @@ void Server::hookupLogger(AOClient* client)
             logger, &ULogger::logKick);
     connect(client, &AOClient::logModcall,
             logger, &ULogger::logModcall);
+    connect(client, &AOClient::clientSuccessfullyDisconnected,
+            this,   &Server::markIDFree);
 }
 
 bool Server::isIPBanned(QHostAddress f_remote_IP)