[Freeciv-Dev] (PR#977) Extended connect dialog

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

I've merged all the connect dialog tickets into the original. Please use
PR#977 for all future correspondence on this.

Now to the comments:

connectdlg_common.c:
 o check coding style. (spaces, comments doing line wraps...)
 o "Owner" as name. use user_username() instead.

don't use /rename anymore (remove the rename code). Instead use /take

there is a bug in the load savegame code. picking an AI player picks the
wrong player.

on PAGE_START, each radio highlights on mouseover. This looks ugly. Think
you could do something about it?

+++ freeciv-cvs-Aug-13/server/plrhand.c 2003-08-13 22:46:05.000000000 +0000
@@ -17,6 +17,7 @@

 #include <assert.h>
 #include <stdarg.h>
+#include <stdio.h>

 #include "diptreaty.h"
 #include "events.h"
@@ -1100,7 +1101,7 @@
   if (pplayer) {
     dest = (struct conn_list*)&pplayer->connections;
   } else {
-    dest = &game.game_connections;
+    dest = &game.est_connections;
   }

   va_start(args, format);
@@ -1119,7 +1120,7 @@
   if (pplayer) {
     dest = (struct conn_list*)&pplayer->connections;
   } else {
-    dest = &game.game_connections;
+    dest = &game.est_connections;
   }

Explain the reason you do this. In my opinion this is wrong.

+#define CHALLENGE_ROOT ".freeciv-tmp"
and sprintf(fn, ".civserverlog%d", server_port);

These are not good names. On windows and especially on VMS.

+++ freeciv-cvs-Aug-13/common/mem.c     2003-08-13 22:45:50.000000000 +0000
@@ -115,7 +115,11 @@
 char *real_mystrdup(const char *str,
                    const char *called_as, int line, const char *file)
 {
-  char *dest = (char *)fc_real_malloc(strlen(str)+1, called_as, line,
   file);
+  char *dest;
+
+  if (!str) return NULL;
+
+  dest = (char*)fc_real_malloc(strlen(str)+1, called_as, line, file);
   /* no need to check whether dest is non-NULL! */
   strcpy(dest, str);
   return dest;

This can be omitted from the patch. This is my old cruft.

+  strcpy(pack.message, "/ score\n");
+  strcpy(pack.message, "/ quit\n");

remove the writespace.

userNB as a variable name... change it.

report.c:1152: warning: no previous prototype for `page_conn_flagged'
stdinhand.c: In function `report_server_options':
stdinhand.c:2668: warning: implicit declaration of function
`page_conn_flagged'

I got this, do you?

connectdlg_common.c:59:bool hoping_for_human_opponents = FALSE;

what's this? cruft?



metaserver: I'm very opposed to this. As I've said before, if you want to
run a serious server (by which I mean advertising on metaserver, etc), then 
you should NOT be running the server from the client. This sort of thing
belongs in a GUI frontend to the server proper. I'm not quite sure that if
one doesn't know enough to run a server proper, he ought to be advertising
on metaserver... Please remove it.

rulesetdirs: I don't like this at all. In my opinion this entirely breaks
the distinction between client and server. Remove it. It doesn't make sense
anyway (rulesets could be anywhere). In a future patch, you may implement 
this as a server only function.

Save: should act just like /save: the server should pick the savegame name
even if we used Save As... before. Save As... should have the previous savegame
name though...

server options: I've been convinced that having a gui for this is ok, but this
implementation makes the front page look way too busy. This should be in a
separate dialog (combined with Game/Server Options). Put a button only on
the newgame page.

Quit: I was, to say the least, quite surprised to find multiple stale
civservers running on by machine after testing. It seems that Quitting out
of the client doesn't kill the civserver. It should. The only time that the
server shouldn't be killed is by using "Disconnect from Game". Also remove
the popup in favor of a append_output_window()

authentication solution: I'll admit that your solution was clever, however, 
I'm uncomfortable with the idea of a separate [tiny] popup for each query.
I don't know yet what to recommend. I'll think about it.

-mike