[Freeciv-Dev] (PR#12605) fix game-loading nation/ruleset hack

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

There is a rather bad hack in the loading of a game through the client 
interface.  The server of course loads the ruleset before loading the 
game, but it doesn't send ruleset info to the client until later, when 
the game starts.  So for ruleset-specific information that is needed 
during the game load (nation names and flags) the server has to send 
this as part of the game-load packet.

This is ugly in several ways.  The main problem is the client has to do 
a manual lookup into the tileset to find the flag sprite.  The server 
doesn't send the alternate flag sprite, and the client doesn't bother 
looking up the final fallback flag sprite, so if the tileset is 
incomplete the flag will not be shown (incorrectly, since it will be 
shown once the game starts).

This patch fixes it simply by sending the ruleset info before the 
game-load packet.  Therefore only nation IDs are sent instead of nation 
names and flag sprite names.

This also allows load_sprite and unload_sprite to be removed from the 
tileset interface.  sprite_exists is unused and can also be removed. 
This is important since these functions allow/encourage users outside of 
the tileset to add on extra sprites that are required by the tileset - 
which is bad since only specialized tilesets will include them.

-jason

Index: client/tilespec.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.c,v
retrieving revision 1.275
diff -u -r1.275 tilespec.c
--- client/tilespec.c   23 Mar 2005 17:49:38 -0000      1.275
+++ client/tilespec.c   23 Mar 2005 20:10:32 -0000
@@ -4111,6 +4111,15 @@
 }
 
 /**************************************************************************
+  Return the sprite for the nation.
+**************************************************************************/
+struct Sprite *get_nation_flag_sprite(struct tileset *t,
+                                     const struct nation_type *nation)
+{
+  return nation ? nation->flag_sprite : NULL;
+}
+
+/**************************************************************************
   Return a "sample" sprite for this city style.
 **************************************************************************/
 struct Sprite *get_sample_city_sprite(struct tileset *t, int city_style)
Index: client/tilespec.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.h,v
retrieving revision 1.136
diff -u -r1.136 tilespec.h
--- client/tilespec.h   23 Mar 2005 17:49:38 -0000      1.136
+++ client/tilespec.h   23 Mar 2005 20:10:32 -0000
@@ -179,6 +179,8 @@
                                  struct citizen_type type,
                                  int citizen_index,
                                  const struct city *pcity);
+struct Sprite *get_nation_flag_sprite(struct tileset *t,
+                                     const struct nation_type *nation);
 struct Sprite *get_sample_city_sprite(struct tileset *t, int city_style);
 struct Sprite *get_arrow_sprite(struct tileset *t);
 struct Sprite *get_tax_sprite(struct tileset *t, Output_type_id otype);
Index: client/gui-gtk-2.0/pages.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/pages.c,v
retrieving revision 1.20
diff -u -r1.20 pages.c
--- client/gui-gtk-2.0/pages.c  11 Mar 2005 17:11:26 -0000      1.20
+++ client/gui-gtk-2.0/pages.c  23 Mar 2005 20:10:32 -0000
@@ -1431,13 +1431,13 @@
  FIXME: this is somewhat duplicated in plrdlg.c, 
         should be somewhere else and non-static
 **************************************************************************/
-static GdkPixbuf *get_flag(char *flag_str)
+static GdkPixbuf *get_flag(const struct nation_type *nation)
 {
   int x0, y0, x1, y1, w, h;
   GdkPixbuf *im, *im2;
   SPRITE *flag;
 
-  flag = load_sprite(tileset, flag_str);
+  flag = get_nation_flag_sprite(tileset, nation);
 
   if (!flag) {
     return NULL;
@@ -1461,7 +1461,6 @@
   im = gdk_pixbuf_new_subpixbuf(sprite_get_pixbuf(flag), x0, y0, w, h);
   im2 = gdk_pixbuf_copy(im);
   g_object_unref(im);
-  unload_sprite(tileset, flag_str);
 
   /* and finaly store the scaled flag pixbuf in the static flags array */
   return im2;
@@ -1480,20 +1479,29 @@
 
   for (i = 0; i < packet->nplayers; i++) {
     GtkTreeIter iter;
-    GdkPixbuf *flag;
+    const char *nation_name;
+
+    if (packet->nations[i] == NO_NATION_SELECTED) {
+      nation_name = "";
+    } else {
+      nation_name = get_nation_name(packet->nations[i]);
+    }
 
     gtk_list_store_append(nation_store, &iter);
     gtk_list_store_set(nation_store, &iter, 
        0, packet->name[i],
-       2, packet->nation_name[i],
+       2, nation_name,
        3, packet->is_alive[i] ? _("Alive") : _("Dead"),
        4, packet->is_ai[i] ? _("AI") : _("Human"), -1);
 
     /* set flag if we've got one to set. */
-    if (strcmp(packet->nation_flag[i], "") != 0) {
-      flag = get_flag(packet->nation_flag[i]);
-      gtk_list_store_set(nation_store, &iter, 1, flag, -1);
-      g_object_unref(flag);
+    if (packet->nations[i] != NO_NATION_SELECTED) {
+      GdkPixbuf *flag = get_flag(get_nation_by_idx(packet->nations[i]));
+
+      if (flag) {
+       gtk_list_store_set(nation_store, &iter, 1, flag, -1);
+       g_object_unref(flag);
+      }
     }
   }
 
Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.223
diff -u -r1.223 capstr.c
--- common/capstr.c     23 Mar 2005 02:09:23 -0000      1.223
+++ common/capstr.c     23 Mar 2005 20:10:32 -0000
@@ -82,7 +82,7 @@
  *     as long as possible.  We want to maintain network compatibility with
  *     the stable branch for as long as possible.
  */
-#define CAPABILITY "+Freeciv.Devel.2004.Mar.22"
+#define CAPABILITY "+Freeciv.Devel.2004.Mar.23"
 
 void init_our_capability(void)
 {
Index: common/fc_types.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/fc_types.h,v
retrieving revision 1.15
diff -u -r1.15 fc_types.h
--- common/fc_types.h   14 Mar 2005 20:26:25 -0000      1.15
+++ common/fc_types.h   23 Mar 2005 20:10:32 -0000
@@ -35,6 +35,7 @@
 
 struct city;
 struct government;
+struct nation_type;
 struct player;
 struct tile;
 struct unit;
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.97
diff -u -r1.97 packets.def
--- common/packets.def  5 Mar 2005 00:06:25 -0000       1.97
+++ common/packets.def  23 Mar 2005 20:10:33 -0000
@@ -193,7 +193,7 @@
 type UNIT              = UINT16
 type TECH              = UINT8
 type UNIT_TYPE         = uint8(Unit_Type_id)
-type NATION            = uint16(Nation_Type_id)
+type NATION            = sint16(Nation_Type_id)
 type GOVERNMENT                = UINT8
 type CONNECTION                = UINT8
 type TEAM              = UINT8
@@ -1252,8 +1252,7 @@
   STRING load_filename[MAX_LEN_PACKET];
   STRING name[MAX_NUM_PLAYERS:nplayers][MAX_LEN_NAME]; 
   STRING username[MAX_NUM_PLAYERS:nplayers][MAX_LEN_NAME]; 
-  STRING nation_name[MAX_NUM_PLAYERS:nplayers][MAX_LEN_NAME]; 
-  STRING nation_flag[MAX_NUM_PLAYERS:nplayers][MAX_LEN_NAME]; 
+  NATION nations[MAX_NUM_PLAYERS:nplayers];
   BOOL is_alive[MAX_NUM_PLAYERS:nplayers];
   BOOL is_ai[MAX_NUM_PLAYERS:nplayers];
 end
Index: server/stdinhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/stdinhand.c,v
retrieving revision 1.390
diff -u -r1.390 stdinhand.c
--- server/stdinhand.c  22 Mar 2005 04:03:35 -0000      1.390
+++ server/stdinhand.c  23 Mar 2005 20:10:34 -0000
@@ -3130,6 +3130,9 @@
   if (load_successful) {
     int i = 0;
 
+    /* We have to send ruleset info before sending the nations, below. */
+    send_rulesets(game.est_connections);
+
     players_iterate(pplayer) {
       if (game.nation_count && is_barbarian(pplayer)) {
        continue;
@@ -3138,12 +3141,9 @@
       sz_strlcpy(packet.name[i], pplayer->name);
       sz_strlcpy(packet.username[i], pplayer->username);
       if (game.nation_count) {
-       sz_strlcpy(packet.nation_name[i], get_nation_name(pplayer->nation));
-       sz_strlcpy(packet.nation_flag[i],
-           get_nation_by_plr(pplayer)->flag_graphic_str);
+       packet.nations[i] = pplayer->nation;
       } else { /* No nations picked */
-       sz_strlcpy(packet.nation_name[i], "");
-       sz_strlcpy(packet.nation_flag[i], "");
+       packet.nations[i] = NO_NATION_SELECTED;
       }
       packet.is_alive[i] = pplayer->is_alive;
       packet.is_ai[i] = pplayer->ai.control;