<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39946 >

Now that the code and symbols are more readable (see PR#39940), several
potential bugs become more apparent.

Fix bad parameters in savegame (from PR#39940).

The number of players is in game.info, so send_game_info() MUST be called
before send_player_info().  Fixed several potential bugs.

Fix old hack by using S_S_GENERATING_WAITING, so named because previous code
called it GAME_GENERATION_STATE, "Waiting", UNUSED_STATE, and "Unknown".

Removed duplicate sends in server/plrhand.c split_player()
Removed duplicate sends in server/stdinhand.c set_command()

Since this is a cleanup, not affecting user visible features, leave the
popdown_races_dialog(), but flag it with a FIXME!

Index: server/srv_main.c
===================================================================
--- server/srv_main.c   (revision 14128)
+++ server/srv_main.c   (working copy)
@@ -836,12 +836,12 @@
   freelog(LOG_DEBUG, "Updatetimeout");
   update_timeout();
 
+  freelog(LOG_DEBUG, "Sendgameinfo");
+  send_game_info(NULL);
+
   freelog(LOG_DEBUG, "Sendplayerinfo");
   send_player_info(NULL, NULL);
 
-  freelog(LOG_DEBUG, "Sendgameinfo");
-  send_game_info(NULL);
-
   freelog(LOG_DEBUG, "Sendyeartoclients");
   send_year_to_clients(game.info.year);
 }
@@ -974,9 +974,10 @@
 
   con_puts(C_OK, _("Starting game."));
 
-  set_server_state(S_S_RUNNING); /* loaded ??? */
+  set_server_state(S_S_GENERATING_WAITING); /* loaded ??? */
   force_end_of_sniff = TRUE;
   send_server_settings(NULL);
+  /* There's no stateful packet set to client until srv_ready(). */
 }
 
 /**************************************************************************
@@ -1205,11 +1206,10 @@
 
   if (S_S_RUNNING == server_state()
       && type != PACKET_PLAYER_READY) {
-    /* HACK: the player_ready packet puts the server into S_S_RUNNING
-     * but doesn't actually start the game.  The game isn't started until
-     * the main loop is re-entered, so kill_dying_players would think
+    /* handle_player_ready() calls start_game(), but the game isn't started
+     * until the main loop is re-entered, so kill_dying_players would think
      * all players are dead.  This should be solved by adding a new
-     * game state S_S_GENERATING. */
+     * game state (now S_S_GENERATING_WAITING). */
     kill_dying_players();
   }
 
Index: server/gamehand.c
===================================================================
--- server/gamehand.c   (revision 14128)
+++ server/gamehand.c   (working copy)
@@ -374,6 +374,10 @@
 /**************************************************************************
   Send game_info packet; some server options and various stuff...
   dest==NULL means game.est_connections
+
+  It may be sent at any time. It MUST be sent before any player info, 
+  as it contains the number of players.  To avoid inconsistency, it
+  SHOULD be sent after rulesets and any other server settings.
 **************************************************************************/
 void send_game_info(struct conn_list *dest)
 {
Index: server/edithand.c
===================================================================
--- server/edithand.c   (revision 14128)
+++ server/edithand.c   (working copy)
@@ -503,11 +503,11 @@
     research->tech_goal = A_UNSET;
   }
 
+  /* Inform everybody about global advances */
+  send_game_info(NULL);
+
   /* send update back to client */
   send_player_info(NULL, pplayer);
-
-  /* Inform everybody about global advances */
-  send_game_info(NULL);
 }
 
 /****************************************************************************
Index: server/meta.c
===================================================================
--- server/meta.c       (revision 14128)
+++ server/meta.c       (working copy)
@@ -235,8 +235,8 @@
   case S_S_OVER:
     sz_strlcpy(state, "Game Ended");
     break;
-  case S_S_GENERATING_WAITING_UNUSED:
-    sz_strlcpy(state, "Unknown");
+  case S_S_GENERATING_WAITING:
+    sz_strlcpy(state, "Generating");
     break;
   }
 
Index: server/sernet.c
===================================================================
--- server/sernet.c     (revision 14128)
+++ server/sernet.c     (working copy)
@@ -1176,7 +1176,7 @@
   case S_S_OVER:
     my_snprintf(status, sizeof(status), "Game over");
     break;
-  case S_S_GENERATING_WAITING_UNUSED:
+  case S_S_GENERATING_WAITING:
     my_snprintf(status, sizeof(status), "Waiting");
   }
 
Index: server/stdinhand.c
===================================================================
--- server/stdinhand.c  (revision 14128)
+++ server/stdinhand.c  (working copy)
@@ -2596,17 +2596,14 @@
 
   if (!check && do_update) {
     send_server_setting(NULL, cmd);
-    send_game_info(NULL);
-    reset_all_start_commands();
-    send_server_info_to_metaserver(META_INFO);
     /* 
      * send any modified game parameters to the clients -- if sent
      * before S_S_RUNNING, triggers a popdown_races_dialog() call
      * in client/packhand.c#handle_game_info() 
      */
-    if (S_S_RUNNING == server_state()) {
-      send_game_info(NULL);
-    }
+    send_game_info(NULL);
+    reset_all_start_commands();
+    send_server_info_to_metaserver(META_INFO);
   }
   return TRUE;
 }
@@ -3290,8 +3287,8 @@
 
   sanity_check();
   
-  send_game_info(game.est_connections);
   send_rulesets(game.est_connections);
+  send_game_info(game.est_connections);
 
   /* Everything seemed to load ok; spread the good news. */
   send_load_game_info(TRUE);
@@ -3739,7 +3736,7 @@
               "to disconnect."));
     return FALSE;
   case S_S_RUNNING:
-  case S_S_GENERATING_WAITING_UNUSED:
+  case S_S_GENERATING_WAITING:
     /* TRANS: given when /start is invoked while the game is running. */
     cmd_reply(CMD_START_GAME, caller, C_FAIL,
               _("Cannot start the game: it is already running."));
Index: server/savegame.c
===================================================================
--- server/savegame.c   (revision 14128)
+++ server/savegame.c   (working copy)
@@ -3616,7 +3616,7 @@
   }
 
   tmp_server_state = (enum server_states)
-    secfile_lookup_int_default(file, S_S_RUNNING, "game.server_state()");
+    secfile_lookup_int_default(file, S_S_RUNNING, "game.server_state");
 
   {
     set_meta_patches_string(secfile_lookup_str_default(file, 
@@ -4301,7 +4301,7 @@
    * game for savegame purposes:
    */
   secfile_insert_int(file, (int) (game.info.is_new_game ? server_state() :
-                                 S_S_RUNNING), "game.server_state()");
+                                 S_S_RUNNING), "game.server_state");
   
   secfile_insert_str(file, get_meta_patches_string(), "game.metapatches");
   secfile_insert_bool(file, game.meta_info.user_message_set,
Index: server/plrhand.c
===================================================================
--- server/plrhand.c    (revision 14128)
+++ server/plrhand.c    (working copy)
@@ -1589,9 +1589,6 @@
     assess_danger_player(pplayer);
   }
 
-  send_game_info(NULL);
-  send_player_info(cplayer, NULL);
-
   return cplayer;
 }
 
@@ -1686,7 +1683,7 @@
 
   cplayer = split_player(pplayer);
 
-  /* So that clients get the correct player_count(): */
+  /* So that clients get the correct number of players */
   send_game_info(NULL);
   
   /* Before units, cities, so clients know name of new nation
Index: common/game.h
===================================================================
--- common/game.h       (revision 14128)
+++ common/game.h       (working copy)
@@ -33,9 +33,10 @@
   DEBUG_LAST
 };
 
+/* used in savegame values */
 enum server_states { 
   S_S_INITIAL = 0, 
-  S_S_GENERATING_WAITING_UNUSED = 1,   /* For savegame compatibility. */
+  S_S_GENERATING_WAITING = 1,
   S_S_RUNNING = 2,
   S_S_OVER = 3,
 };
@@ -44,7 +45,7 @@
 enum client_states { 
   C_S_INITIAL = 0,
   C_S_PREPARING = 1,
-  C_S_STARTING = 2,
+  C_S_STARTING_UNUSED = 2,
   C_S_RUNNING = 3,
   C_S_OVER = 4,
 };
Index: client/packhand.c
===================================================================
--- client/packhand.c   (revision 14128)
+++ client/packhand.c   (working copy)
@@ -1463,7 +1463,9 @@
   game.government_when_anarchy
     = government_by_number(game.info.government_when_anarchy_id);
   game.player_ptr = player_by_number(game.info.player_idx);
+
   if (C_S_PREPARING == client_state()) {
+    /* FIXME: only for change in nations */
     popdown_races_dialog();
   }
   boot_help = (can_client_change_view()
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to