<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40195 >

 If modpack has less than MAX_PLAYERS playable nations, server can run
out of nations to assign to players and crash. Attached patch prevents
several such cases.

 - There's no duplicate code for handling both start methods. #40194
should be fixed so that duplicate code is not needed. /start should be
correctly handled, Start-button not.
 - I have not yet investigated civil war player creation. It will
probably require fixing too.


 - ML

diff -Nurd -X.diff_ignore freeciv/server/connecthand.c freeciv/server/connecthand.c
--- freeciv/server/connecthand.c	2008-04-03 20:14:42.000000000 +0300
+++ freeciv/server/connecthand.c	2008-04-07 00:14:17.000000000 +0300
@@ -475,7 +475,8 @@
 
       if (NULL == pplayer) {
         /* no uncontrolled player found */
-        if (game.info.nplayers >= game.info.max_players) {
+        if (game.info.nplayers >= game.info.max_players
+            || game.info.nplayers - server.nbarbarians >= server.playable_nations) {
           return FALSE;
         }
         assert(game.info.nplayers < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c	2008-04-01 22:40:44.000000000 +0300
+++ freeciv/server/ruleset.c	2008-04-07 00:17:33.000000000 +0300
@@ -2490,7 +2490,9 @@
 
     pl->is_playable = secfile_lookup_bool_default(file, TRUE,
 						  "%s.is_playable", sec[i]);
-
+    if (pl->is_playable) {
+      server.playable_nations++;
+    }
 
     /* Check barbarian type. Default is "None" meaning not a barbarian */    
     barb_type = secfile_lookup_str_default(file, "None",
@@ -3554,6 +3556,7 @@
 
   ruleset_data_free();
   reset_player_nations();
+  server.playable_nations = 0;
 
   openload_ruleset_file(&techfile, "techs");
   load_tech_names(&techfile);
@@ -3611,6 +3614,10 @@
 
   /* Build AI unit class cache corresponding to loaded rulesets */
   unit_class_ai_init();
+
+  /* We may need to adjust number of AI players if number of available
+   * nations changed */
+  aifill(game.info.aifill);
 }
 
 /**************************************************************************
diff -Nurd -X.diff_ignore freeciv/server/srv_main.c freeciv/server/srv_main.c
--- freeciv/server/srv_main.c	2008-04-01 22:40:44.000000000 +0300
+++ freeciv/server/srv_main.c	2008-04-07 00:15:10.000000000 +0300
@@ -1592,6 +1592,9 @@
 {
   int limit = MIN(amount, game.info.max_players);
 
+  /* Limit to nations provided by ruleset */
+  limit = MIN(limit, server.playable_nations);
+
   if (!game.info.is_new_game || S_S_INITIAL != server_state()) {
     return;
   }
@@ -2145,6 +2148,7 @@
 void server_game_init(void)
 {
   /* was redundantly in game_load() */
+  server.playable_nations = 0;
   server.nbarbarians = 0;
   server.identity_number = IDENTITY_NUMBER_SKIP;
 
diff -Nurd -X.diff_ignore freeciv/server/srv_main.h freeciv/server/srv_main.h
--- freeciv/server/srv_main.h	2008-03-10 19:54:21.000000000 +0200
+++ freeciv/server/srv_main.h	2008-04-07 00:11:53.000000000 +0300
@@ -60,6 +60,7 @@
  *
  * TODO: Lots more variables could be added here. */
 extern struct civserver {
+  int playable_nations;
   int nbarbarians;
 
   /* this counter creates all the city and unit numbers used.
diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c	2008-04-06 11:24:53.000000000 +0300
+++ freeciv/server/stdinhand.c	2008-04-07 00:40:09.000000000 +0300
@@ -818,13 +818,13 @@
 }
 
 /**************************************************************************
-...
+ Creates named AI player
 **************************************************************************/
 static bool create_ai_player(struct connection *caller, char *arg, bool check)
 {
   PlayerNameStatus PNameStatus;
   struct player *pplayer = NULL;
-   
+
   if (S_S_INITIAL != server_state())
   {
     cmd_reply(CMD_CREATE, caller, C_SYNTAX,
@@ -840,14 +840,19 @@
     }
   } players_iterate_end;
 
-  /* game.info.max_players is a limit on the number of non-observer players.
-   * MAX_NUM_PLAYERS is a limit on all players. */
-  if (game.info.nplayers >= MAX_NUM_PLAYERS) {
-    if (NULL == pplayer) {
+  if (NULL == pplayer) {
+    /* Check that we are not going over max players setting */
+    if (game.info.nplayers >= game.info.max_players) {
       cmd_reply(CMD_CREATE, caller, C_FAIL,
 	        _("Can't add more players, server is full."));
       return FALSE;
     }
+    /* Check that we have nations available */
+    if (game.info.nplayers - server.nbarbarians >= server.playable_nations) {
+      cmd_reply(CMD_CREATE, caller, C_FAIL,
+	        _("Can't add more players, not enough nations."));
+      return FALSE;
+    }
   }
 
   if ((PNameStatus = test_player_name(arg)) == PNameEmpty)
@@ -2966,12 +2971,13 @@
    * they first release. */
   if (!pplayer && !pconn->playing
       && (game.info.nplayers >= game.info.max_players
-          || game.info.nplayers >= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS)) {
+          || game.info.nplayers - server.nbarbarians >= server.playable_nations)) {
     cmd_reply(CMD_TAKE, caller, C_FAIL,
               _("There is no free player slot for %s"),
               pconn->username);
     goto end;
   }
+  assert(game.info.nplayers < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
 
   res = TRUE;
   if (check) {
@@ -3798,6 +3804,10 @@
       cmd_reply(CMD_START_GAME, caller, C_FAIL,
 		_("Not enough players, game will not start."));
       return FALSE;
+    } else if (game.info.nplayers - server.nbarbarians >= server.playable_nations) {
+      cmd_reply(CMD_START_GAME, caller, C_FAIL,
+		_("Not enough nations for all players, game will not start."));
+      return FALSE;
     } else if (check) {
       return TRUE;
     } else if (!caller) {
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to