[Freeciv-Dev] (PR#7378) Vote command for server

["Reinier Post" <rp@xxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7378 >

Now some comments on the way Per's vote works.

Good to require that a full turn must pass.
This makes it hard to cheat, unless a /vote 'set timeout 1'
somehow slips through.  We must be really sure that no such abuse is
possible before removing /fix.

I think the syntax is complicated.  Whence the single quotes?
Is it impossible to vote on cutting O'Hara, or is that name
disallowed now?

The vote numbers are unfriendly.  ("Motion 1836, discussed last Tuesday, 
all in favour please raise your hands.")  /vote lists them,
but I'd much prefer to just say

  /vote timeout 20

to vote to have the timeout set to 20.  No countervotes.
For commands, you can have

  /vote cut Hannibal

and something like (pick one)

  /no cut
  /dont cut
  /unvote cut

to place a countervote.

Wouldn't this suffice just as well?

I think changes should take effect on a majority vote.
For settings: the value most voted for, if it is unique;
for commands: if it got more votes than countervotes.
I don't think the players who abstain from voting must be counted;
that way /vote becomes useless when many players are around.

About the patch: good to see that you fixed these other issues.
On pubserver I use a simple handle_stdin_input patch that is
somewhat related to this one: it fixes the problem that players
tend to type

  /start ?

without really meaning to issue the start command.

I like the idea to have a "just checking syntax" mode for 
handle_stdin_input.

diff -Nurd -x'.when-*' -x'.newest-*' -Xdiff_ignore 
../freeciv.orig/./server/stdinhand.c ./server/stdinhand.c
--- ../freeciv.orig/./server/stdinhand.c        Sun Nov 30 11:22:48 2003
+++ ./server/stdinhand.c        Thu Dec 18 12:17:55 2003
@@ -1108,9 +1108,9 @@
   {"metaconnection",   ALLOW_HACK,
    "metaconnection u|up\n"
    "metaconnection d|down\n"
-   "metaconnection ?",
+   "metaconnection",
    N_("Control metaserver connection."),
-   N_("'metaconnection ?' reports on the status of the connection to 
metaserver.\n"
+   N_("'metaconnection' reports on the status of the connection to 
metaserver.\n"
       "'metaconnection down' or 'metac d' brings the metaserver connection 
down.\n"
       "'metaconnection up' or 'metac u' brings the metaserver connection up.")
   },
@@ -1541,7 +1541,7 @@
 static void metaconnection_command(struct connection *caller, char *arg)
 {
   if ((*arg == '\0') ||
-      (0 == strcmp (arg, "?"))) {
+      (0 == strcmp (arg, ""))) {
     if (server_is_open) {
       cmd_reply(CMD_METACONN, caller, C_COMMENT,
                _("Metaserver connection is open."));
@@ -3057,6 +3057,22 @@
 }
 
 /**************************************************************************
+  Cutting away trailing whitespace.
+**************************************************************************/
+static void cut_trailing_whitespace(char *str)
+{
+  int i;
+
+  freelog(LOG_DEBUG,"cut_trailing_whitespace(str='%s')",str);
+
+  for (i = strlen(str)-1; i >= 0 && my_isspace(str[i]); i--) {
+    str[i] = '\0';
+  }
+
+  freelog(LOG_DEBUG,"cut_trailing_whitespace: returning '%s'",str);
+}
+
+/**************************************************************************
   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
@@ -3065,8 +3081,7 @@
 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;
-  int i;
+      *cptr_s, *cptr_d;
   enum command_id cmd;
 
   /* notify to the server console */
@@ -3103,6 +3118,9 @@
    * skipped leading whitespace, the SERVER_COMMAND_PREFIX and any
    * other non-alphanumeric characters.
    */
+
+  /* try to interpret the first word as the name of a command */
+
   for (cptr_d = command; *cptr_s != '\0' && my_isalnum(*cptr_s) &&
       cptr_d < command+sizeof(command)-1; cptr_s++, cptr_d++)
     *cptr_d=*cptr_s;
@@ -3120,6 +3138,7 @@
        _("Unknown command.  Try '%shelp'."), caller?"/":"");
     return;
   }
+
   if (!may_use(caller, cmd)) {
     assert(caller != NULL);
     cmd_reply(cmd, caller, C_FAIL,
@@ -3127,20 +3146,30 @@
     return;
   }
 
+  /* the rest of the line are the command arguments */
+
   for (; *cptr_s != '\0' && my_isspace(*cptr_s); cptr_s++) {
     /* nothing */
   }
   sz_strlcpy(arg, cptr_s);
 
   cut_comment(arg);
+  cut_trailing_whitespace(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';
+  /*
+   * multiplayer users are known to type things like "/start ?",
+   * so lines ending in "?" should not actually be taken as commands;
+   * we'll give help instead
+   */
+  if (strlen(arg) > 0 && arg[strlen(arg)-1] == '?') {
+    arg[strlen(arg)-1] = '\0';
+    if (cmd == CMD_SET) {
+      explain_option(caller,arg);
+    } else {
+      show_help(caller, command);
+    }
+    return;
+  }
 
   if (commands[cmd].level > ALLOW_INFO) {
     /*
@@ -3255,7 +3284,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) {