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

On 01/04/2008, Marko Lindqvist  wrote:
>   S2_1:
>    1. Connect client A to server
>    2. A: /set aifill 30
>    3. Connect client B to server (connection is not automatically attached)
>    4. B: /take -
>
>   S2_2 and TRUNK:
>    1. Connect client A to server
>    2. A: /set maxplayers 1
>    3. Connect client B to server (connection is not automatically attached)
>    4. B: /take -

 S2_1 committed: r14540

 Minimally tested TRUNK patch (probably applies to S2_2 also) attached.


 - ML

diff -Nurd -X.diff_ignore freeciv/server/connecthand.c freeciv/server/connecthand.c
--- freeciv/server/connecthand.c	2008-03-19 04:30:55.000000000 +0200
+++ freeciv/server/connecthand.c	2008-04-02 00:02:56.000000000 +0300
@@ -434,11 +434,28 @@
 }
 
 /**************************************************************************
+  Search for first uncontrolled player
+**************************************************************************/
+struct player *find_uncontrolled_player(void)
+{
+  players_iterate(played) {
+    if (!played->is_connected && !played->was_created) {
+      return played;
+    }
+  } players_iterate_end;
+
+  return NULL;
+}
+
+/**************************************************************************
   Setup pconn as a client connected to pplayer:
   Updates pconn, pplayer->connections, pplayer->is_connected.
 
   If pplayer is NULL, take the next available player that is not connected.
   Note "observer" connections do not count for is_connected.
+  Note take_command() needs to know if this function will success before
+       it's time to call this. Keep take_command() checks in sync when
+       modifying this.
 **************************************************************************/
 bool attach_connection_to_player(struct connection *pconn,
                                  struct player *pplayer,
@@ -448,23 +465,15 @@
     assert(NULL != pplayer);
   } else {
     if (NULL == pplayer) {
-      /* search for first uncontrolled player */
-      players_iterate(played) {
-        if (!played->is_connected && !played->was_created) {
-          pplayer = played;
-          break;
-        }
-      } players_iterate_end;
+      /* search for uncontrolled player */
+      pplayer = find_uncontrolled_player();
 
       if (NULL == pplayer) {
         /* no uncontrolled player found */
-        if (game.info.nplayers >= game.info.max_players
-            || game.info.nplayers >= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS) {
-          /* the latter test is probably redundant, and should never happen;
-           * no remaining player slots.
-           */
+        if (game.info.nplayers >= game.info.max_players) {
           return FALSE;
         }
+        assert(game.info.nplayers < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);
 
         /* add new player */
         pplayer = &game.players[game.info.nplayers];
diff -Nurd -X.diff_ignore freeciv/server/connecthand.h freeciv/server/connecthand.h
--- freeciv/server/connecthand.h	2008-03-10 20:08:03.000000000 +0200
+++ freeciv/server/connecthand.h	2008-04-02 00:03:01.000000000 +0300
@@ -34,6 +34,7 @@
 void send_conn_info(struct conn_list *src, struct conn_list *dest);
 void send_conn_info_remove(struct conn_list *src, struct conn_list *dest);
 
+struct player *find_uncontrolled_player(void);
 bool attach_connection_to_player(struct connection *pconn,
                                  struct player *pplayer,
                                  bool observing);
diff -Nurd -X.diff_ignore freeciv/server/stdinhand.c freeciv/server/stdinhand.c
--- freeciv/server/stdinhand.c	2008-03-30 20:36:32.000000000 +0300
+++ freeciv/server/stdinhand.c	2008-04-02 00:04:57.000000000 +0300
@@ -2930,7 +2930,11 @@
   }
 
   if (strcmp(arg[i], "-") == 0) {
-    pplayer = NULL;
+    /* Find first uncontrolled player. This will return NULL if there is
+     * no free players at the moment. Later call to
+     * attach_connection_to_player() will create new player for such NULL
+     * cases. */
+    pplayer = find_uncontrolled_player();
   } else if (!(pplayer = find_player_by_name_prefix(arg[i], &match_result))) {
     cmd_reply_no_such_player(CMD_TAKE, caller, arg[i], match_result);
     goto end;
@@ -2953,6 +2957,22 @@
     goto end;
   }
 
+  if (!pplayer) {
+  }
+
+  /* Make sure there is free player slot if there is need to
+   * create new player. This is necessary for previously
+   * detached connections only. Others can reuse the slot
+   * they first release. */
+  if (!pplayer && !pconn->playing
+      && (game.info.nplayers >= game.info.max_players
+          || game.info.nplayers >= MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS)) {
+    cmd_reply(CMD_TAKE, caller, C_FAIL,
+              _("There is no free player slot for %s"),
+              pconn->username);
+    goto end;
+  }
+
   res = TRUE;
   if (check) {
     goto end;
@@ -3015,31 +3035,40 @@
   } players_iterate_end;
 
   /* now attach to new player */
-  attach_connection_to_player(pconn, pplayer, FALSE);
-  pplayer = pconn->playing; /* In case pplayer was NULL. */
+  res = attach_connection_to_player(pconn, pplayer, FALSE);
+
+  /* Check aifill even if attach failed. Maybe we already detached. */
   aifill(game.info.aifill);
 
-  /* inform about the status before changes */
-  cmd_reply(CMD_TAKE, caller, C_OK, _("%s now controls %s (%s, %s)"),
-            pconn->username,
-            player_name(pplayer),
-            is_barbarian(pplayer)
-            ? _("Barbarian")
-            : pplayer->ai.control
+  if (res) {
+    /* Successfully attached */
+    pplayer = pconn->playing; /* In case pplayer was NULL. */
+
+    /* inform about the status before changes */
+    cmd_reply(CMD_TAKE, caller, C_OK, _("%s now controls %s (%s, %s)"),
+              pconn->username,
+              player_name(pplayer),
+              is_barbarian(pplayer)
+              ? _("Barbarian")
+              : pplayer->ai.control
               ? _("AI")
               : _("Human"),
-            pplayer->is_alive
-            ? _("Alive")
-            : _("Dead"));
+              pplayer->is_alive
+              ? _("Alive")
+              : _("Dead"));
 
-  /* if pplayer wasn't /created, and we're still in pregame, change its name */
-  if (!pplayer->was_created && is_newgame && !pplayer->nation) {
-    sz_strlcpy(pplayer->name, pconn->username);
-  }
+    /* if pplayer wasn't /created, and we're still in pregame, change its name */
+    if (!pplayer->was_created && is_newgame && !pplayer->nation) {
+      sz_strlcpy(pplayer->name, pconn->username);
+    }
 
-  /* aitoggle the player back to human as necessary. */
-  if (pplayer->ai.control && game.info.auto_ai_toggle) {
-    toggle_ai_player_direct(NULL, pplayer);
+    /* aitoggle the player back to human as necessary. */
+    if (pplayer->ai.control && game.info.auto_ai_toggle) {
+      toggle_ai_player_direct(NULL, pplayer);
+    }
+  } else {
+    cmd_reply(CMD_TAKE, caller, C_FAIL, _("%s failed to attach to any player"),
+              pconn->username);
   }
 
   if (S_S_RUNNING == server_state()) {
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to