<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 >
> [wsimpson - Mon Feb 25 13:02:09 2008]:
>
> Now that I've had some sleep....
>
> Madeline Book wrote:
> > Yes, I was afraid of something like that. So what are the
> > semantics of client.playing?
>
> It is set from game.info.player_idx -- exclusively.
So I supposed it should be set for clients connecting to
a server somewhere remotely (e.g. a to play a game online).
> > Is it supposed to be NULL
> > when the client is an observer (unlike aconnection.player
> > apparently) and during pregame before the user has picked
> > his nation and set his player name?
>
> Had never noticed aconnection.player, and have no idea about
> its purpose.
I thought you were intending to use client.playing to replace
it. The question is now why prefer client.playing over
aconnection.player? Maybe adding client.playing was not
necessary to begin with (as opposed to just improving
the use of aconnection.player)?
> Certainly, aconnection.player is a bad idea. That isn't
> properly updated
> as players are removed (before start) -- unlike client.playing.
>
> Yet Another Data Duplication headache.
YADDH(tm). ;)
> > What then is supposed
> > to be passed to popup_races_dialog?
> >
> Heck, you shouldn't be able to select a race before
> player is assigned.
What about when connecting to a remote server? Newly
connected clients should have a player. It seems that
there is just a problem in this case of client.playing
not being set to the client's player pointer.
Maybe it would be a good idea to include a multiplayer
connection scenario when testing changes to the server's
multiplayer data structures (as opposed to only testing
with AIs and the fork'd server).
> Compare XAW:
>
> if (client.playing) {
> popup_races_dialog(client.playing);
> }
>
> With GTK2:
>
> if (aconnection.player) {
> popup_races_dialog(client.playing);
> } else if (game.info.is_new_game) {
> send_chat("/take -");
> popup_races_dialog(client.playing);
> }
It would seem to make more sense to change aconnection
.player to client.playing in the above (or the converse
;)). It looks like some incomplete conversion when both
occur in the code like this.
> I added the last call in PR#39927 back in December, both 2.1
> and 2.2, so that's not within the past 2 weeks:
>
> send_chat("/take -");
> + popup_races_dialog(game.player_ptr);
Hmm, so there it was assumed that game.player_ptr would
already be a pointer to a valid player (i.e. not NULL).
> This aconnection.player change appeared in PR#16459, so perhaps
> Per can
> explain:
>
> static void pick_nation_callback(GtkWidget *w, gpointer data)
> {
> - if (game.player_ptr) {
> + if (aconnection.player) {
> popup_races_dialog(game.player_ptr);
> + } else if (game.info.is_new_game) {
> + send_chat("/take -");
> }
> }
I agree this is a curious change. What is the intended
difference in use of aconnection.player and game.player_ptr?
Is game.player_ptr only valid after the game starts (or
more precisely after the client's player has been fixed
after final nation selection)? This would seem to imply
that client.playing is not right to be passed to
popup_races_dialog in pregame, it should be aconnection
.player. But I'm just guessing.
> For connection dialog, another call to popup_races_dialog()
> with another parameter was added even earlier in PR#15688,
> so perhaps Jason can explain:
>
> +static void conn_menu_nation_chosen(GtkMenuItem *menuitem,
> gpointer data)
> +{
> + popup_races_dialog(conn_menu_player);
> +}
>
> So, a third static copy of player, always a joy to debug! :-(
The global (static) variables are a pain; I agree a better
programming style should be used if possible (e.g. gtk
gives you a "userdata" pointer that you can use to pass
arbitrary data to callbacks).
> > No, I just connected to a server running on my local
> > machine and tried to start the game (incidentally doing
> > it this way the "Start" button is also inactive, which
> > might indicate that there is some deeper problem).
> >
> Probably just sensitive to whether you've picked a Nation yet.
But if I use the main page's "Start New Game" button it
is sensitive. As I understand it, pressing "Start" without
having picked your nation makes the server automatically
assign you a nation at random (when the game starts).
So "Start" should be sensitive in all player cases (i.e.
not for observers or detached connections).
> > 1. Start a server process (e.g. with ./ser in the build
> > directory).
> >
> > 2. Start a client process (e.g. with "./civ -a"
> > in the build directory).
> >
> > 3. Client started in (2) connects to the server started
> > in (1).
> >
> Not on my setup. It hangs in some silly reverse DNS lookup,
> because I'm
> running behind a firewall -- as all good developers always do!
A reverse DNS lookup for 127.0.0.1 ? ;)
But anyway I find it hard to believe you would have such trouble
running a server first and then separately running a client to
connect to it on the same machine. Perhaps you could try
running the client like "./civ -a -s 192.168.0.5" (replace
192.168.0.5 of course with the appropriate address).
> Depending on a reverse DNS lookup is a bad idea, as there are
> many users that run NAT with RFC-1918 addresses, and it is an
> unreasonable load on (wearing another hat) our root servers.
Doesn't gethostbyaddr respect this? Maybe you should report
this to the various libc developers for freeciv's target
platforms.
> Depending on a DNS lookup of any kind completing is a
> terrible idea....
There is no access control based on hostname in S2_2, so
at worst the cosmetic feature of seeing the client's hostname
in the server would not be possible, if the reverse DNS
lookup times-out.
> But that's another ticket. I'm pretty sure there's an existing one.
PR#?
> > 4. Now as the client, press the button labelled "Pick Nation".
> >
> Sounds like that should be sensitive to whether you have a player yet.
I would think clients connecting to a remote server have
a player once they successfully connect (baring of course
being shunted into observer/detached status due to the
maxplayers setting or similar).
> > 5. The races (a.k.a. nations) dialog pops-up.
> >
> > 6. Click any nation in the list (e.g. Assyrian).
> >
> > 7. The button labelled "Ok" becomes active.
> >
> > 8. Press the button labelled "Ok".
> >
> > 9. The assertion failure occurs.
> >
>
>
> > A few weeks ago branch S2_2 did not experience this assertion
> > failure. As I was using it in this exact manner for working
> > on the editor.
> >
> Since it doesn't occur with the more usual New Game page,
> I suggest that you try it for your Editor experiments, until
> this is tracked down.
What has now been revealed, is that essentially hosting a server
and having clients connect to it is broken. So only single
player game starting works at the moment. I would hope that
this assertion failure is fixed quickly so that playing on
remote servers (i.e. multiplayer) is possible.
> Or you could try checking out earlier revisions by binary
> search until you find the revision number that caused the
> problem....
>
> Until somebody does that, it's unlikely to be fixed.
Why not the architect of client.playing? That person would
probably have the best grasp of the data depedencies and
side-effects inherent in juggling around client.playing,
aconnection.player, game.player_ptr, and related code.
Sure, I can just use my hackfix (git is woderful!) so that
I can continue my other work until an more appropraite
solution is agreed upon. ;)
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
