[Freeciv-Dev] (PR#15688) gtk2 client needs pregame observer support

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [jdorje - Wed Mar 01 17:35:22 2006]:

> However this is problematic since there is no corresponding button to
> reverse observing.  After you /observe your player is automatically
> deleted.  To reclaim a player I think you have to:
> 
>   /create some-name
>   /take some-name
>   /aitoggle some-name
> 
> which is so complex that we cannot expect anyone to actually figure it
> out, aside from which you have to pick "some-name" to be a name that's
> not already in use or it won't work.  The end result is that if a user
> presses "observe" by accident they've effectively fubared their game.
> 
> Thus, before this is finalized we need to change "/take" so that if
> given no parameters it creates and takes a player for you.  Then the
> observe button can be toggled onto a "take player" button when you are
> observing.

This patch should nearly finalize the pregame controls. It solves the
above problems as well as fixing a few other bugs.

* The take command now accepts '-' in place of the player name, in which
case the conn is assigned to a newly-created player (this is actually
fairly trivial, since attach_connection_to_player already accepts NULL).
* The "Observe" button in pregame toggles to a "Take player" button when
the player is observing or detached.  Clicking the "Take player" button
simply does "/take -".
* A bug is fixed.  Previously doing "/observe" then "/create foo" then
"/take foo" would leave you *observing* foo.
* Another bug is fixed.  The "Start" button in the pregame dialog does
nothing when you are detached.  Now it is desensitized in this case.

Remaining issues:

* There is no way to start a game when all players are AIs.  Running
/start is simply ignored since you are not attached to a player.
* The commands may not work properly in the client when
!game.is_new_game.  This needs more testing.

-jason

Index: server/commands.c
===================================================================
--- server/commands.c   (revision 11727)
+++ server/commands.c   (working copy)
@@ -180,7 +180,9 @@
    N_("Take over a player's place in the game."),
    N_("Only the console and connections with cmdlevel 'hack' can force "
       "other connections to take over a player. If you're not one of these, "
-      "only the <player-name> argument is allowed")
+      "only the <player-name> argument is allowed.  If '-' is given for the "
+      "player name and the connection does not already control a player, one "
+      "is created and assigned to the connection.")
   },
   {"observe",    ALLOW_INFO,
    /* TRANS: translate text between [] only */
Index: server/stdinhand.c
===================================================================
--- server/stdinhand.c  (revision 11727)
+++ server/stdinhand.c  (working copy)
@@ -89,9 +89,6 @@
                          bool check);
 static bool set_away(struct connection *caller, char *name, bool check);
 
-static bool is_allowed_to_take(struct player *pplayer, bool will_obs, 
-                               char *msg);
-
 static bool observe_command(struct connection *caller, char *name, bool check);
 static bool take_command(struct connection *caller, char *name, bool check);
 static bool detach_command(struct connection *caller, char *name, bool check);
@@ -2584,7 +2581,7 @@
 {
   const char *allow;
 
-  if (!pplayer) {
+  if (!pplayer && will_obs) {
     /* Observer */
     if (!(allow = strchr(game.allow_take, (game.info.is_new_game ? 'O' : 
'o')))) {
       if (will_obs) {
@@ -2597,6 +2594,9 @@
       }
       return FALSE;
     }
+  } else if (!pplayer && !will_obs) {
+    /* Auto-taking a new player */
+    return game.info.is_new_game && server_state == PRE_GAME_STATE;
   } else if (is_barbarian(pplayer)) {
     if (!(allow = strchr(game.allow_take, 'b'))) {
       if (will_obs) {
@@ -2844,7 +2844,7 @@
   char buf[MAX_LEN_CONSOLE_LINE], *arg[2], msg[MAX_LEN_MSG];
   bool is_newgame = server_state == PRE_GAME_STATE && game.info.is_new_game;
   enum m_pre_result match_result;
-  struct connection *pconn = NULL;
+  struct connection *pconn = caller;
   struct player *pplayer = NULL;
   bool res = FALSE;
   
@@ -2879,16 +2879,13 @@
     i++; /* found a conn, now reference the second argument */
   }
 
-  if (!(pplayer = find_player_by_name_prefix(arg[i], &match_result))) {
+  if (strcmp(arg[i], "-") == 0) {
+    pplayer = NULL;
+  } 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;
   }
 
-  /* if we don't assign other connections to players, assign us to be pconn. */
-  if (!pconn) {
-    pconn = caller;
-  }
-
   /******** PART II: do the attaching ********/
 
   /* check allowtake for permission */
@@ -2898,11 +2895,12 @@
   }
 
   /* taking your own player makes no sense. */
-  if (pconn->player == pplayer && !pconn->observer) {
+  if ((pplayer && !pconn->observer && pconn->player == pplayer)
+      || (!pplayer && !pconn->observer && pconn->player)) {
     cmd_reply(CMD_TAKE, caller, C_FAIL, _("%s already controls %s"),
-              pconn->username, pplayer->name);
+              pconn->username, pconn->player->name);
     goto end;
-  } 
+  }
 
   res = TRUE;
   if (check) {
@@ -2920,32 +2918,38 @@
 
   /* if we're taking another player with a user attached, 
    * forcibly detach the user from the player. */
-  conn_list_iterate(pplayer->connections, aconn) {
-    if (!aconn->observer) {
-      if (server_state == RUN_GAME_STATE) {
-        send_game_state(aconn->self, CLIENT_PRE_GAME_STATE);
-       send_rulesets(aconn->self);
-       send_server_settings(aconn->self);
+  if (pplayer) {
+    conn_list_iterate(pplayer->connections, aconn) {
+      if (!aconn->observer) {
+       if (server_state == RUN_GAME_STATE) {
+         send_game_state(aconn->self, CLIENT_PRE_GAME_STATE);
+         send_rulesets(aconn->self);
+         send_server_settings(aconn->self);
+       }
+       notify_conn(aconn->self, NULL, E_CONNECTION,
+                   _("being detached from %s."), pplayer->name);
+       unattach_connection_from_player(aconn);
+       send_conn_info(aconn->self, game.est_connections);
       }
-      notify_conn(aconn->self, NULL, E_CONNECTION,
-                 _("being detached from %s."), pplayer->name);
-      unattach_connection_from_player(aconn);
-      send_conn_info(aconn->self, game.est_connections);
-    }
-  } conn_list_iterate_end;
+    } conn_list_iterate_end;
+  }
 
   /* if the connection is already attached to a player,
    * unattach and cleanup old player (rename, remove, etc) */
   if (pconn->player) {
     char name[MAX_LEN_NAME];
 
-    /* if a pconn->player is removed, we'll lose pplayer */
-    sz_strlcpy(name, pplayer->name);
+    if (pplayer) {
+      /* if a pconn->player is removed, we'll lose pplayer */
+      sz_strlcpy(name, pplayer->name);
+    }
 
     detach_command(NULL, pconn->username, FALSE);
 
-    /* find pplayer again, the pointer might have been changed */
-    pplayer = find_player_by_name(name);
+    if (pplayer) {
+      /* find pplayer again; the pointer might have been changed */
+      pplayer = find_player_by_name(name);
+    }
   }
 
   /* we don't want the connection's username on another player */
@@ -2956,11 +2960,13 @@
   } players_iterate_end;
 
   /* now attach to new player */
+  pconn->observer = FALSE; /* do this before attach! */
   attach_connection_to_player(pconn, pplayer);
   send_conn_info(pconn->self, game.est_connections);
+  pplayer = pconn->player; /* In case pplayer was NULL. */
  
   /* if pplayer wasn't /created, and we're still in pregame, change its name */
-  if (!pplayer->was_created && is_newgame) {
+  if (!pplayer->was_created && is_newgame && !pplayer->nation) {
     sz_strlcpy(pplayer->name, pconn->username);
   }
 
Index: client/gui-gtk-2.0/gui_main.c
===================================================================
--- client/gui-gtk-2.0/gui_main.c       (revision 11727)
+++ client/gui-gtk-2.0/gui_main.c       (working copy)
@@ -1386,8 +1386,17 @@
     }
 
     gtk_stockbutton_set_label(ready_button, text);
+  } else {
+    gtk_stockbutton_set_label(ready_button, _("_Start"));
   }
+  gtk_widget_set_sensitive(ready_button, (game.player_ptr != NULL));
 
+  if (game.player_ptr && !aconnection.observer) {
+    gtk_stockbutton_set_label(take_button, _("_Observe"));
+  } else {
+    gtk_stockbutton_set_label(take_button, _("_Take player"));
+  }
+
   gtk_tree_view_column_set_visible(record_col, (with_ggz || in_ggz));
   gtk_tree_view_column_set_visible(rating_col, (with_ggz || in_ggz));
 
Index: client/gui-gtk-2.0/pages.c
===================================================================
--- client/gui-gtk-2.0/pages.c  (revision 11729)
+++ client/gui-gtk-2.0/pages.c  (working copy)
@@ -57,7 +57,7 @@
 GtkTreeViewColumn *rating_col, *record_col;
 
 static GtkWidget *start_options_table;
-GtkWidget *ready_button;
+GtkWidget *take_button, *ready_button;
 
 static GtkWidget *scenario_description;
 
@@ -998,9 +998,13 @@
 /**************************************************************************
   Called when "observe" is clicked.
 **************************************************************************/
-static void observe_callback(GtkWidget *w, gpointer data)
+static void take_callback(GtkWidget *w, gpointer data)
 {
-  send_chat("/observe");
+  if (game.player_ptr && !aconnection.observer) {
+    send_chat("/observe");
+  } else {
+    send_chat("/take -");
+  }
 }
 
 /**************************************************************************
@@ -1436,10 +1440,10 @@
                   G_CALLBACK(pick_nation_callback), NULL);
   gtk_container_add(GTK_CONTAINER(bbox), button);
 
-  button = gtk_stockbutton_new(GTK_STOCK_ZOOM_IN, _("_Observe"));
-  g_signal_connect(button, "clicked",
-                  G_CALLBACK(observe_callback), NULL);
-  gtk_container_add(GTK_CONTAINER(bbox), button);
+  take_button = gtk_stockbutton_new(GTK_STOCK_ZOOM_IN, _("_Observe"));
+  g_signal_connect(take_button, "clicked",
+                  G_CALLBACK(take_callback), NULL);
+  gtk_container_add(GTK_CONTAINER(bbox), take_button);
 
   ready_button = gtk_stockbutton_new(GTK_STOCK_EXECUTE, _("_Ready"));
   g_signal_connect(ready_button, "clicked",
Index: client/gui-gtk-2.0/pages.h
===================================================================
--- client/gui-gtk-2.0/pages.h  (revision 11727)
+++ client/gui-gtk-2.0/pages.h  (working copy)
@@ -20,7 +20,7 @@
 #include "pages_g.h"
 
 extern GtkWidget *start_message_area;
-extern GtkWidget *ready_button;
+extern GtkWidget *take_button, *ready_button;
 extern GtkTreeViewColumn *rating_col, *record_col;
 
 GtkWidget *create_main_page(void);