Fix #5173

Do not modify the max-players-in-game parameter to interpret it in server_config.cpp, instead do the interpretation in server_lobby.cpp

The issue was caused by the modified parameter being then saved in the XML, causing issues later on.

This commit also addresses a similar issue with the min-start-game-players parameter

src/network/protocols/server_lobby.cpp

@@ -874,17 +874,22 @@ void ServerLobby::asynchronousUpdate()
     {
         if (ServerConfig::m_owner_less)
         {
+            // Ensure that a game can auto-start if the server meets the config's starting limit or if it's already full.
+            int starting_limit = std::min((int)ServerConfig::m_min_start_game_players, (int)ServerConfig::m_server_max_players);
+            if (ServerConfig::m_max_players_in_game > 0) // 0 here means it's not the limit
+                starting_limit = std::min(starting_limit, (int)ServerConfig::m_max_players_in_game);
+
             unsigned players = 0;
             STKHost::get()->updatePlayers(&players);
-            if (((int)players >= ServerConfig::m_min_start_game_players ||
+            if (((int)players >= starting_limit ||
                 m_game_setup->isGrandPrixStarted()) &&
                 m_timeout.load() == std::numeric_limits<int64_t>::max())
             {
                 m_timeout.store((int64_t)StkTime::getMonoTimeMs() +
                     (int64_t)
                     (ServerConfig::m_start_game_counter * 1000.0f));
             }
-            else if ((int)players < ServerConfig::m_min_start_game_players &&
+            else if ((int)players < starting_limit &&
                 !m_game_setup->isGrandPrixStarted())
             {
                 resetPeersReady();
@@ -894,7 +899,7 @@ void ServerLobby::asynchronousUpdate()
             }
             if (m_timeout.load() < (int64_t)StkTime::getMonoTimeMs() ||
                 (checkPeersReady(true/*ignore_ai_peer*/) &&
-                (int)players >= ServerConfig::m_min_start_game_players))
+                (int)players >= starting_limit))
             {
                 resetPeersReady();
                 startSelection();
@@ -4355,7 +4360,12 @@ std::set<std::shared_ptr<STKPeer>> ServerLobby::getSpectatorsByLimit()
     auto peers = STKHost::get()->getPeers();
     std::set<std::shared_ptr<STKPeer>> always_spectate_peers;
 
-    unsigned player_limit = ServerConfig::m_max_players_in_game;
+    unsigned player_limit = ServerConfig::m_server_max_players;
+    // If the server has an in-game player limit lower than the lobby limit, apply it,
+    // A value of 0 for this parameter means no limit.
+    if (ServerConfig::m_max_players_in_game > 0)
+        player_limit = std::min(player_limit, (unsigned)ServerConfig::m_max_players_in_game);
+
     // only 10 players allowed for battle or soccer
     if (RaceManager::get()->isBattleMode() || RaceManager::get()->isSoccerMode())
         player_limit = std::min(player_limit, 10u);

src/network/server_config.cpp

@@ -341,8 +341,10 @@ void loadServerLobbyFromConfig()
         m_server_max_players > 10)
         m_server_max_players = 10;
 
-    m_max_players_in_game = (m_max_players_in_game <= 0) ? m_server_max_players :
-        std::min(m_max_players_in_game, m_server_max_players);
+    // Parameters should only be sanity checked, not modified for interpretation (see #5173).
+    // A parameter of 0 here means no limit is to be applied.
+    if (m_max_players_in_game < 0)
+        m_max_players_in_game = 0;
 
     if (m_ipv6_connection)
     {
@@ -360,8 +362,6 @@ void loadServerLobbyFromConfig()
     }
     if (m_owner_less)
     {
-        if (m_min_start_game_players > m_max_players_in_game)
-            m_min_start_game_players = 1;
         if (!m_live_players)
             m_team_choosing = false;
         m_server_configurable = false;