[Freeciv-Dev] (PR#2282) echo command lines only when successful

["Reinier Post via RT" <rt@xxxxxxxxxxxxxx>]

This is a patch to improve the feedback on commands, which was affected
by recent patches, and also reported in Jitterbug (PR#2240 and PR#2252).

The new behaviour is debatable, but I can't see how to improve further
without making the code even uglier.  Perhaps the feedback for
some of the commands can be improved.

As a side effect, I removed the buffer allargs from handle_stdin_input.
Its existence seems due to a mistake in reading the code.

-- 
Reinier

diff -Nurd -x'.when-*' -x'.newest-*' -Xdiff_ignore 
/home/freeciv/.freeciv/code/unpatched/beta/freeciv-1.14.0-beta1/freeciv/./server/stdinhand.c
 ./server/stdinhand.c
--- 
/home/freeciv/.freeciv/code/unpatched/beta/freeciv-1.14.0-beta1/freeciv/./server/stdinhand.c
        Wed Oct 16 11:43:00 2002
+++ ./server/stdinhand.c        Tue Oct 29 13:34:31 2002
@@ -1349,8 +1349,36 @@
 /**************************************************************************
   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->name : _("(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,
@@ -1362,18 +1390,24 @@
                   cmd == CMD_UNRECOGNIZED ? _("(unknown)") :
                        "(?!?)";  /* this case is a bug! */
 
+  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);
-    /* cc: to the console - testing has proved it's too verbose - rp
+    /* 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);
   }
-
-  if (rfc_status == C_OK) {
-    notify_player(NULL, _("Game: %s"), line);
-  }
 }
 
 /**************************************************************************
@@ -1411,6 +1445,9 @@
                             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);
@@ -1418,6 +1455,8 @@
 
 /**************************************************************************
   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, ...)
@@ -1426,6 +1465,9 @@
                      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);
@@ -1776,6 +1818,9 @@
 
   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));
 }
 
 
@@ -2664,7 +2709,6 @@
   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++) {
@@ -2717,7 +2761,6 @@
   op = &settings[cmd];
 
   do_update = FALSE;
-  buffer[0] = '\0';
 
   switch (op->type) {
   case SSET_BOOL:
@@ -2735,9 +2778,12 @@
        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;
@@ -2757,9 +2803,11 @@
        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;
       }
     }
@@ -2777,18 +2825,17 @@
        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) {
     /* canonify map generator settings (all of which are int) */
     adjust_terrain_param();
@@ -3001,15 +3048,10 @@
 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->name, 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,
@@ -3066,28 +3108,24 @@
   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->name : _("(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:
@@ -3188,7 +3226,7 @@
     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) {