[Freeciv-Dev] (PR#2252) Chatline commands echoed only partially

["jjc@xxxxxxxxxxxxxxxxxx" <jjc@xxxxxxxxxxxxxxxxxx>]

Updating patch to work with current cvs.  This patch is too help fix the 
problem of:

When someone gives a buggy server command like

/ai <nonexistent player>

in the client chatline, everybody sees the command echoed,
but only the person who issued the command sees the error
message. The result might be confusion about what commands
have been given or not.

This patch only echos successfull commands.

The use of a global variable cmdline_to_echo seems ugly.

Should we echo caller->player->name or caller->username 
or both?

-- 
Josh Cogliati


Index: server/stdinhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/stdinhand.c,v
retrieving revision 1.290
diff -U9 -r1.290 stdinhand.c
--- server/stdinhand.c  2003/08/28 15:54:11     1.290
+++ server/stdinhand.c  2003/08/31 20:11:46
@@ -1427,43 +1427,77 @@
   } else {
     return sset_is_to_client(option_idx)
       || may_set_option(caller, option_idx);
   }
 }
 
 /**************************************************************************
   feedback related to server commands
   caller == NULL means console.
-  No longer duplicate all output to console.
+**************************************************************************/
 
+/**************************************************************************
+  Auxiliary function that reports the command issued back to everybody
+  if it affects the game; this helps human players learn from each other
+  and displays abuse.  The implementation is ugly:
+  we rely on handle_stdin_input() to pass the command buffer in
+  cmdline_to_echo, which exists for no other purpose than to be read here.
+**************************************************************************/
+static char cmdline_to_echo[MAX_LEN_CONSOLE_LINE];
+
+static void cmd_echo_to_all_if_ok(enum command_id cmd,struct connection 
*caller,
+                                 enum rfc_status rfc_status)
+{
+  if (rfc_status == C_OK && commands[cmd].level > ALLOW_INFO) {
+    /* the command affects the game */
+
+    /* notify everybody */
+    notify_player(NULL, "%s: '%s'",
+                 caller ? caller->username : _("(server prompt)"),
+       cmdline_to_echo);
+
+    /* cc: the console */
+    if (caller) {
+      con_write(C_COMMENT,"%s", cmdline_to_echo);
+    }
+  }
+}
+
+/**************************************************************************
   This lowlevel function takes a single line; prefix is prepended to line.
 **************************************************************************/
 static void cmd_reply_line(enum command_id cmd, struct connection *caller,
                           enum rfc_status rfc_status, const char *prefix,
                           const char *line)
 {
   const char *cmdname = cmd < CMD_NUM ? commands[cmd].name :
                   cmd == CMD_AMBIGUOUS ? _("(ambiguous)") :
                   cmd == CMD_UNRECOGNIZED ? _("(unknown)") :
                        "(?!?)";  /* this case is a bug! */
 
-  if (caller) {
-    notify_conn(&caller->self, "/%s: %s%s", cmdname, prefix, line);
-    /* cc: to the console - testing has proved it's too verbose - rp
-    con_write(rfc_status, "%s/%s: %s%s", caller->name, cmdname, prefix, line);
-    */
-  } else {
-    con_write(rfc_status, "%s%s", prefix, line);
-  }
-
-  if (rfc_status == C_OK) {
+  if (rfc_status == C_OK && commands[cmd].level > ALLOW_INFO) {
+    /* notify everybody */
     notify_player(NULL, _("Game: %s"), line);
+    /* including the console */
+    con_write(rfc_status, _("Game: %s"), line);
   }
+  else
+    /* notify only the issuer */
+    if (caller) {
+      /* issuer is not the console */
+      notify_conn(&caller->self, "/%s: %s%s", cmdname, prefix, line);
+      /* don't cc: to the console - testing has proved it too verbose - rp
+        con_write(rfc_status, "%s/%s: %s%s", caller->name, cmdname, prefix, 
line);
+      */
+    } else {
+      /* issuer is the console */
+      con_write(rfc_status, "%s%s", prefix, line);
+    }
 }
 
 /**************************************************************************
   va_list version which allow embedded newlines, and each line is sent
   separately. 'prefix' is prepended to every line _after_ the first line.
 **************************************************************************/
 static void vcmd_reply_prefix(enum command_id cmd, struct connection *caller,
                              enum rfc_status rfc_status, const char *prefix,
                              const char *format, va_list ap)
@@ -1489,33 +1523,41 @@
 static void cmd_reply_prefix(enum command_id cmd, struct connection *caller,
                             enum rfc_status rfc_status, const char *prefix,
                             const char *format, ...)
      fc__attribute((format (printf, 5, 6)));
 static void cmd_reply_prefix(enum command_id cmd, struct connection *caller,
                             enum rfc_status rfc_status, const char *prefix,
                             const char *format, ...)
 {
   va_list ap;
+
+  cmd_echo_to_all_if_ok(cmd, caller, rfc_status);
+
   va_start(ap, format);
   vcmd_reply_prefix(cmd, caller, rfc_status, prefix, format, ap);
   va_end(ap);
 }
 
 /**************************************************************************
   var-args version as above, no prefix
+  also displays the original command line if the command was
+    non-informational and successful
 **************************************************************************/
 static void cmd_reply(enum command_id cmd, struct connection *caller,
                      enum rfc_status rfc_status, const char *format, ...)
      fc__attribute((format (printf, 4, 5)));
 static void cmd_reply(enum command_id cmd, struct connection *caller,
                      enum rfc_status rfc_status, const char *format, ...)
 {
   va_list ap;
+
+  cmd_echo_to_all_if_ok(cmd, caller, rfc_status);
+
   va_start(ap, format);
   vcmd_reply_prefix(cmd, caller, rfc_status, "", format, ap);
   va_end(ap);
 }
 
 /**************************************************************************
 ...
 **************************************************************************/
 static void cmd_reply_no_such_player(enum command_id cmd,
@@ -1892,18 +1934,21 @@
   if (!pplayer)
   {
     cmd_reply(CMD_CREATE, caller, C_FAIL,
              _("Error creating new AI player: %s."), arg);
     return;
   }
 
   pplayer->ai.control = TRUE;
   set_ai_level_directer(pplayer, game.skill_level);
+  cmd_reply(CMD_CREATE, caller, C_OK,
+       _("Created new AI player '%s' with skill level '%s'."),
+               pplayer->name, name_of_skill_level(game.skill_level));
 }
 
 
 /**************************************************************************
 ...
 **************************************************************************/
 static void remove_player(struct connection *caller, char *arg)
 {
   enum m_pre_result match_result;
@@ -2868,19 +2913,18 @@
 /******************************************************************
   ...
 ******************************************************************/
 static void set_command(struct connection *caller, char *str) 
 {
   char command[MAX_LEN_CONSOLE_LINE], arg[MAX_LEN_CONSOLE_LINE], *cptr_s, 
*cptr_d;
   int val, cmd;
   struct settings_s *op;
   bool do_update;
-  char buffer[500];
 
   for (cptr_s = str; *cptr_s != '\0' && !is_ok_opt_name_char(*cptr_s);
        cptr_s++) {
     /* nothing */
   }
 
   for(cptr_d=command;
       *cptr_s != '\0' && is_ok_opt_name_char(*cptr_s);
       cptr_s++, cptr_d++) {
@@ -2921,39 +2965,41 @@
     cmd_reply(CMD_SET, caller, C_BOUNCE,
              _("This setting is protected from being modified after the "
                 "game has started."));
     return;
   }
 
   op = &settings[cmd];
 
   do_update = FALSE;
-  buffer[0] = '\0';
 
   switch (op->type) {
   case SSET_BOOL:
     if (sscanf(arg, "%d", &val) != 1) {
       cmd_reply(CMD_SET, caller, C_SYNTAX, _("Value must be an integer."));
     } else if (val != 0 && val != 1) {
       cmd_reply(CMD_SET, caller, C_SYNTAX,
                _("Value out of range (minimum: 0, maximum: 1)."));
     } else {
       char *reject_message = NULL;
       bool b_val = (val != 0);
 
       if (settings[cmd].bool_validate
          && !settings[cmd].bool_validate(b_val, &reject_message)) {
        cmd_reply(CMD_SET, caller, C_SYNTAX, reject_message);
       } else {
        *(op->bool_value) = b_val;
-       my_snprintf(buffer, sizeof(buffer),
+       if (sset_is_to_client(cmd)) {
+         cmd_reply(CMD_SET, caller, C_OK,
                    _("Option: %s has been set to %d."), op->name,
                    *(op->bool_value) ? 1 : 0);
+       }
+       do_update = TRUE;
       }
     }
     break;
 
   case SSET_INT:
     if (sscanf(arg, "%d", &val) != 1) {
       cmd_reply(CMD_SET, caller, C_SYNTAX, _("Value must be an integer."));
     } else if (val < op->int_min_value || val > op->int_max_value) {
       cmd_reply(CMD_SET, caller, C_SYNTAX,
@@ -2961,50 +3007,51 @@
                op->int_min_value, op->int_max_value);
     } else {
       char *reject_message = NULL;
 
       if (settings[cmd].int_validate
          && !settings[cmd].int_validate(val, &reject_message)) {
        cmd_reply(CMD_SET, caller, C_SYNTAX, reject_message);
       } else {
        *(op->int_value) = val;
-       my_snprintf(buffer, sizeof(buffer),
+       if (sset_is_to_client(cmd)) {
+         cmd_reply(CMD_SET, caller, C_OK,
                    _("Option: %s has been set to %d."), op->name,
                    *(op->int_value));
+       }
        do_update = TRUE;
       }
     }
     break;
 
   case SSET_STRING:
     if (strlen(arg) >= op->string_value_size) {
       cmd_reply(CMD_SET, caller, C_SYNTAX,
                _("String value too long.  Usage: set <option> <value>."));
     } else {
       char *reject_message = NULL;
 
       if (settings[cmd].string_validate
          && !settings[cmd].string_validate(arg, &reject_message)) {
        cmd_reply(CMD_SET, caller, C_SYNTAX, reject_message);
       } else {
        strcpy(op->string_value, arg);
-       my_snprintf(buffer, sizeof(buffer),
+       if (sset_is_to_client(cmd)) {
+         cmd_reply(CMD_SET, caller, C_OK,
                    _("Option: %s has been set to \"%s\"."), op->name,
                    op->string_value);
+       }
+       do_update = TRUE;
       }
     }
     break;
   }
 
-  if (strlen(buffer) > 0 && sset_is_to_client(cmd)) {
-    notify_player(NULL, "%s", buffer);
-  }
-
   if (do_update) {
     /* 
      * send any modified game parameters to the clients -- if sent
      * before RUN_GAME_STATE, triggers a popdown_races_dialog() call
      * in client/packhand.c#handle_game_info() 
      */
     if (server_state == RUN_GAME_STATE) {
       send_game_info(NULL);
     }
@@ -3681,27 +3728,22 @@
 /**************************************************************************
   Handle "command input", which could really come from stdin on console,
   or from client chat command, or read from file with -r, etc.
   caller==NULL means console, str is the input, which may optionally
   start with SERVER_COMMAND_PREFIX character
 **************************************************************************/
 void handle_stdin_input(struct connection *caller, char *str)
 {
   char command[MAX_LEN_CONSOLE_LINE], arg[MAX_LEN_CONSOLE_LINE],
-      allargs[MAX_LEN_CONSOLE_LINE], *cptr_s, *cptr_d;
+      *cptr_s, *cptr_d;
   int i;
   enum command_id cmd;
 
-  /* notify to the server console */
-  if (caller) {
-    con_write(C_COMMENT, "%s: '%s'", caller->username, str);
-  }
-
   /* if the caller may not use any commands at all, don't waste any time */
   if (may_use_nothing(caller)) {
     cmd_reply(CMD_HELP, caller, C_FAIL,
        _("Sorry, you are not allowed to use server commands."));
      return;
   }
 
   /* Is it a comment or a blank line? */
   /* line is comment if the first non-whitespace character is '#': */
@@ -3746,40 +3788,36 @@
     assert(caller != NULL);
     cmd_reply(cmd, caller, C_FAIL,
              _("You are not allowed to use this command."));
     return;
   }
 
   for (; *cptr_s != '\0' && my_isspace(*cptr_s); cptr_s++) {
     /* nothing */
   }
+
   sz_strlcpy(arg, cptr_s);
 
   cut_comment(arg);
 
-  /* keep this before we cut everything after a space */
-  sz_strlcpy(allargs, cptr_s);
-  cut_comment(allargs);
-
   i=strlen(arg)-1;
   while(i>0 && my_isspace(arg[i]))
     arg[i--]='\0';
-
-  if (commands[cmd].level > ALLOW_INFO) {
-    /*
-     * this command will affect the game - inform all players
-     *
-     * use command,arg instead of str because of the trailing
-     * newline in str when it comes from the server command line
-     */
-    notify_player(NULL, "%s: '%s %s'",
-      caller ? caller->username : _("(server prompt)"), command, arg);
+  
+  /*
+   * store the part of the command that is actually used,
+   *  for cmd_echo_to_all_if_ok() to report, if necessary
+   */
+  sz_strlcpy(cmdline_to_echo, command);
+  if (arg[0]) {
+    sz_strlcat(cmdline_to_echo, " ");
   }
+  sz_strlcat(cmdline_to_echo, arg);
 
   switch(cmd) {
   case CMD_REMOVE:
     remove_player(caller,arg);
     break;
   case CMD_RENAME:
     rename_player(caller,arg);
     break;
   case CMD_SAVE:
@@ -3886,19 +3924,19 @@
     con_set_style(!con_get_style());
     break;
   case CMD_CMDLEVEL:
     cmdlevel_command(caller,arg);
     break;
   case CMD_FIRSTLEVEL:
     firstlevel_command(caller);
     break;
   case CMD_TIMEOUT:
-    timeout_command(caller, allargs);
+    timeout_command(caller, arg);
     break;
   case CMD_START_GAME:
     if (server_state==PRE_GAME_STATE) {
 
       /* Sanity check scenario */
       if (game.is_new_game) {
        if (map.fixed_start_positions
            && game.max_players > map.num_start_positions) {
          freelog(LOG_VERBOSE, "Reduced maxplayers from %i to %i to fit "