[Freeciv-Dev] Re: (PR#4760) take, observe, detach

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Jason Short wrote:

> "The problem: init_tech (which marks all techs given in the ruleset
> unknown, makes A_NONE and gives init techs) is only called in a new
> game and if after all players chose their nations. So this leaves the
> time window from the time a player connects (and this is published to
> all other players) till the time this player disconnects before the
> init_tech is called. In this time window the tech data is unset. This
> is the problem that the previous patch should have fixed. However the
> previous patch was too restrictive."

The core of the problem is that in establish_new_connection the 
send_player_info is only sent out when server_state==RUN_GAME_STATE [1], 
but in lost_connection_to_client() it is always sent out [2].  This 
means you'll get buggy behavior when the client disconnects but not when 
it connects.

Either way, the checks in these two functions should be consistent.  In 
this patch when disconnecting the player_info packet isn't sent unless 
the game is in RUN_GAME_STATE.  And the bad client logic trying to tell 
whether to read the tech info is simply dropped.  This should fix all of 
the pregame bugs we've had so far.

Should this also include GAME_OVER_STATE?  It's not possible for new 
players to connect at this point, right?  But we still want to send out 
info about disconnecting players, I'd think.

[1] http://www.freeciv.org/lxr/source/server/connecthand.c#L131
[2] http://www.freeciv.org/lxr/source/server/connecthand.c#L591

jason

Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.323
diff -u -r1.323 packhand.c
--- client/packhand.c   2003/08/01 19:58:47     1.323
+++ client/packhand.c   2003/08/08 03:01:37
@@ -1360,11 +1360,7 @@
 
   pplayer->is_connected = pinfo->is_connected;
 
-  if (pplayer->is_connected
-      || get_client_state() == CLIENT_GAME_RUNNING_STATE
-      || get_client_state() == CLIENT_GAME_OVER_STATE) {
-    new_tech = read_player_info_techs(pplayer, pinfo->inventions);
-  }
+  new_tech = read_player_info_techs(pplayer, pinfo->inventions);
 
   poptechup = (pplayer->research.researching != pinfo->researching
                || pplayer->ai.tech_goal != pinfo->tech_goal);
Index: server/connecthand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/connecthand.c,v
retrieving revision 1.11
diff -u -r1.11 connecthand.c
--- server/connecthand.c        2003/08/04 14:57:42     1.11
+++ server/connecthand.c        2003/08/08 03:01:37
@@ -124,6 +124,8 @@
     attach_connection_to_player(pconn, pplayer);
 
     if (server_state == RUN_GAME_STATE) {
+      /* Player and other info is only updated when the game is running.
+       * This is also true in lost_connection_to_client(). */
       send_packet_generic_empty(pconn, PACKET_FREEZE_HINT);
       send_rulesets(dest);
       send_all_info(dest);
@@ -590,7 +592,11 @@
   unattach_connection_from_player(pconn);
 
   send_conn_info_remove(&pconn->self, &game.est_connections);
-  send_player_info(pplayer, NULL);
+  if (server_state == RUN_GAME_STATE) {
+    /* Player info is only updated when the game is running.  See 
+     * establish_new_connection(). */
+    send_player_info(pplayer, NULL);
+  }
   notify_if_first_access_level_is_available();
 
   /* Cancel diplomacy meetings */