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

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Aug 07, 2003 at 08:02:19PM -0700, Jason Short wrote:
> 
> 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.

Well, the first patch certainly seems to fix the observe bug, but I'm
unsure what the connecthand.c patch accomplishes. Why shouldn't new player
info get sent to everybody when a connection is lost whether or not it's in
RUN_GAME_STATE? How is this inconsistent? The two situations are different.
The server sends player info to one player in establish_new_connection()
and to all the other players in lost_connection_to_client().
 
> 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.

well, ideally GAME_OVER_STATE should last as briefly as possible before the
(not-as-yet-realized) server switches back to PRE_GAME_STATE and waits for
a new game. As long as it doesn't last to long, we can certainly prohibit
logins (just don't let sniff_packets() run until we get back to pregame).

-mike

> ===================================================================
> 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 */