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

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Aug 27, 2003 at 09:44:43PM -0700, Daniel L Speyer wrote:
> Mike Kaufman wrote:
> 
> >connectdlg_common.c:
> > o check coding style. (spaces, comments doing line wraps...)
> > o "Owner" as name. use user_username() instead.
> >
> I know there's a good reason I went to Owner, though I can't remember 
> what it is at the moment.  If nothing else, it makes it clear who's 
> running a multi-player game. but that's not the only reason.

well, until you remember, change it.

> >don't use /rename anymore (remove the rename code). Instead use /take
> >  
> >
> Has this code stabilized enough to be worth syncing with?  Last I 
> looked, there was a new way to do it every week and I just stuck with 
> what worked.  Is there a problem with rename?

yes and yes.
 
> >there is a bug in the load savegame code. picking an AI player picks the
> >wrong player.
> >  
> >
> Can you reproduce it?

sure. start a new game with aifill 4. save after a turn. end the game and
load the savegame. pick the third, fourth, or fifth player. Resume game. do
/list. See that you are assigned the second player. I imagine that /rename
is at fault here.

> >Explain the reason you do this. In my opinion this is wrong.
> >  
> This is needed to make sure everyone gets everything while a game is 
> being loaded.  Some important messages were being dropped, though I 
> don't remember which.

hmm, then notify_conn should be used, not this.
 
> >+#define CHALLENGE_ROOT ".freeciv-tmp"
> >and sprintf(fn, ".civserverlog%d", server_port);
> >
> >These are not good names. On windows and especially on VMS.
> >  
> We run on VMS?!

yes. see freeciv/vms. the problem is the period.

> On windows, they just look a little unixy, but no harm done.  If we want 
> to hide them on windows, we'll have to do that specially (not me though, 
> I don't have windows, and I stay away from code I can't test).
> 
> >userNB as a variable name... change it.
> >  
> You mean uberNB?  Do you think really_big_notebook is better?

yes and not necessarily.
 
> >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.
> >  
> Just because someone prefers a GUI doesn't make them an ignoramous, and 
> even if they are, they should still be able to marginally endanger their 
> own data by advertising a server!  This isn't a disk partitioning tool, 
> it's a game -- it should be easy to play.

I agree with all your reasoning and I think you're missing the point.
Please remember what the original purpose of this patch is: single-player.
 
> The function is called from the server.  If you think it would go better 
> in some random server file, I suppose it could move, butt all the 
> related functions are in common.  Also, rulesets can't be "anywhere", 
> they can only be in searched directories.  This is a good reason to 
> leave the code together -- so that if the search algorythm is changed, 
> it will give the correct list.

well, you would be wrong.
% cd freeciv ; cp -a data/civ2 .. ; ser
> rulesetdir ../civ2
Ruleset directory set to "../civ2"

and frankly, I think this is a bug in the implementation. The user should
be able to provide an arbitrary absolute path to a ruleset.

and no. common functions belong in common/ and this is not a common function.
It should never ever be called from the client.

> "Save" and "Save as" are well established in the GUI world and anyone 
> with any GUI experience knows what they mean.  Violating that would be 
> like using "+" for multiplication.

I understand your reasoning. This contradicts the freeciv implementation of
/save. There is a reason why we have a dating suffix.
 
> >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.
> >  

> I'll agree it should be prettier, but hiding everything isn't the right 
> way either.  I tried it.  I think it makes sense to get this patch done 
> and then tweak the UI.

no it doesn't. The UI is the main point of this patch. It will be done
right before it goes in.
 
> Also, I am convinced that it's better to err on the side of exposing to 
> much.  Games don't intimidate people, but the vast majority of our 
> players don't know about the vast majority of our features (check any 
> freeciv.org poll).  Making things easy to play with is how people will 
> learn.

Redundant interfaces --- which is what this is --- make for a unprofessional-
looking game. Crappy-looking interfaces also do this.

> >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
> >
> Can you work out how this happened?  I thought all those bugs were fixed.

I tried and was unable to. hmm.
 
> Meanwhile, how do you use authentication?  I never tested that code 
> becaus eI couldn't figure it out.

./autogen.sh --enable-client=gtk --enable-auth && make

-mike