[Freeciv-Dev] (PR#13453) playable_nation_count cleanups

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

This patch does a lot of cleanups related to
game.control.playable_nation_count.

The main purpose of the patch is to remove practically all references to
this value.  All iterations over "playable" nations now iterate over all
nations and filter by is_nation_playable.  Only
nation.c:is_nation_playable() and ruleset.c:load_nation_names() still
use the value.

To make these changes however lead me into two major cleanups.

1.  I changed pick_available_nation rather drastically.  It now
considers nation groups just like select_random_nation does.
select_random_nation is removed; pick_available_nation is made public
and used in its stead.

2.  In the GTK2 client, I removed select_random_nation.  Instead if
there's no nation selected in the dialog we just tell the server to pick
a random nation for us.

I couldn't test the latter fully because of PR#13451 however.

-jason

Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.528
diff -p -u -r1.528 packhand.c
--- client/packhand.c   9 Jul 2005 17:46:07 -0000       1.528
+++ client/packhand.c   11 Jul 2005 23:30:02 -0000
@@ -2034,7 +2034,6 @@ void handle_ruleset_control(struct packe
   game.control = *packet;
   governments_alloc(packet->government_count);
   nations_alloc(packet->nation_count);
-  game.control.playable_nation_count = packet->playable_nation_count;
   city_styles_alloc(packet->styles_count);
   tileset_alloc_city_tiles(tileset, game.control.styles_count);
 }
Index: client/gui-gtk-2.0/dialogs.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/dialogs.c,v
retrieving revision 1.110
diff -p -u -r1.110 dialogs.c
--- client/gui-gtk-2.0/dialogs.c        11 Jul 2005 07:33:52 -0000      1.110
+++ client/gui-gtk-2.0/dialogs.c        11 Jul 2005 23:30:03 -0000
@@ -91,8 +91,6 @@ static GtkWidget *unit_select_view;
 static GtkTreePath *unit_select_path;
 static struct tile *unit_select_ptile;
 
-static void select_random_race(void);
-  
 static void create_races_dialog(struct player *pplayer);
 static void races_destroy_callback(GtkWidget *w, gpointer data);
 static void races_response(GtkWidget *w, gint response, gpointer data);
@@ -1548,19 +1546,16 @@ void popup_unit_select_dialog(struct til
   Inserts apropriate gtk_tree_view into races_nation_list[i]
   If group == NULL, create a list of all nations
 +****************************************************************/
-static GtkWidget* create_list_of_nations_in_group(struct nation_group* group, 
int index)
+static GtkWidget* create_list_of_nations_in_group(struct nation_group* group,
+                                                 int index)
 {
   GtkWidget *sw;
   GtkListStore *store;
   GtkWidget *list;
-
   GtkTreeSelection *select;
-  
   GtkCellRenderer *render;
   GtkTreeViewColumn *column;
 
-  int i;
-  
   store = gtk_list_store_new(5, G_TYPE_INT, G_TYPE_BOOLEAN,
       GDK_TYPE_PIXBUF, G_TYPE_STRING, G_TYPE_STRING);
   gtk_tree_sortable_set_sort_column_id(GTK_TREE_SORTABLE(store),
@@ -1595,33 +1590,34 @@ static GtkWidget* create_list_of_nations
   g_object_set(render, "style", PANGO_STYLE_ITALIC, NULL);
 
   /* Populate nation list store. */
-  for (i = 0; i < game.control.playable_nation_count; i++) {
-    struct nation_type *nation;
+  nations_iterate(pnation) {
     struct sprite *s;
     bool used;
     GdkPixbuf *img;
     GtkTreeIter it;
     GValue value = { 0, };
 
-    nation = get_nation_by_idx(i);
-    
-    if (group != NULL && !nation_in_group(nation, group->name)) {
+    if (!is_nation_playable(pnation->index) || pnation->is_unavailable) {
+      continue;
+    }
+
+    if (group != NULL && !nation_in_group(pnation, group->name)) {
       continue;
     }
 
     gtk_list_store_append(store, &it);
 
-    s = crop_blankspace(get_nation_flag_sprite(tileset, i));
+    s = crop_blankspace(get_nation_flag_sprite(tileset, pnation->index));
     img = sprite_get_pixbuf(s);
-    used = nation->is_unavailable || nation->is_used;
-    gtk_list_store_set(store, &it, 0, i, 1, used, 2, img, -1);
+    used = pnation->is_used;
+    gtk_list_store_set(store, &it, 0, pnation->index, 1, used, 2, img, -1);
     free_sprite(s);
 
     g_value_init(&value, G_TYPE_STRING);
-    g_value_set_static_string(&value, nation->name);
+    g_value_set_static_string(&value, pnation->name);
     gtk_list_store_set_value(store, &it, 3, &value);
     g_value_unset(&value);
-  }  
+  } nations_iterate_end;
   return sw;
 }
 
@@ -1867,8 +1863,6 @@ void popup_races_dialog(struct player *p
   if (!races_shell) {
     create_races_dialog(pplayer);
     gtk_window_present(GTK_WINDOW(races_shell));
-
-    select_random_race();
   }
 }
 
@@ -1954,45 +1948,6 @@ static void select_random_leader(void)
   g_free(name);
 }
 
-/****************************************************************
-  Selectes a random race and the appropriate city style.
-  Updates the gui elements and the selected_* variables.
- *****************************************************************/
-static void select_random_race(void)
-{
-  GtkTreeModel *model;
-
-  model = gtk_tree_view_get_model(GTK_TREE_VIEW(races_nation_list[0]));
-
-  /* This has a possibility of infinite loop in case
-   * game.control.playable_nation_count < game.info.nplayers. */
-  while (TRUE) {
-    GtkTreePath *path;
-    GtkTreeIter it;
-    int nation;
-
-    nation = myrand(game.control.playable_nation_count);
-
-    path = gtk_tree_path_new();
-    gtk_tree_path_append_index(path, nation);
-
-    if (gtk_tree_model_get_iter(model, &it, path)) {
-      gboolean chosen;
-
-      gtk_tree_model_get(model, &it, 1, &chosen, -1);
-
-      if (!chosen) {
-       gtk_tree_view_set_cursor(GTK_TREE_VIEW(races_nation_list[0]), path,
-           NULL, FALSE);
-       gtk_tree_path_free(path);
-       return;
-      }
-    }
-
-    gtk_tree_path_free(path);
-  }
-}
-
 /**************************************************************************
   ...
  **************************************************************************/
@@ -2044,9 +1999,6 @@ void races_toggles_set_sensitive(void)
       gtk_tree_path_free(path);
     }
   }
-  if (changed) {
-       select_random_race();
-  }
 }
 
 /**************************************************************************
@@ -2064,9 +2016,7 @@ static void races_nation_callback(GtkTre
     gtk_tree_model_get(model, &it, 0, &selected_nation, 1, &chosen, -1);
     nation = get_nation_by_idx(selected_nation);
 
-    if (chosen) {
-      select_random_race();
-    } else {
+    {
       int cs, i, j;
       GtkTreePath *path;
      
@@ -2186,7 +2136,11 @@ static void races_response(GtkWidget *w,
     const char *s;
 
     if (selected_nation == -1) {
-      append_output_window(_("You must select a nation."));
+      dsend_packet_nation_select_req(&aconnection,
+                                    races_player->player_no,
+                                    NO_NATION_SELECTED,
+                                    FALSE, "", 0);
+      popdown_races_dialog();
       return;
     }
 
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.332
diff -p -u -r1.332 citytools.c
--- server/citytools.c  4 Jul 2005 17:48:38 -0000       1.332
+++ server/citytools.c  11 Jul 2005 23:30:10 -0000
@@ -1193,11 +1193,17 @@ void handle_unit_enter_city(struct unit 
   
   if (is_capital(pcity)
       && city_list_size(cplayer->cities) >= game.info.civilwarsize
-      && game.info.nplayers < game.control.playable_nation_count
       && game.info.civilwarsize < GAME_MAX_CIVILWARSIZE
       && get_num_human_and_ai_players() < MAX_NUM_PLAYERS
       && civil_war_triggered(cplayer)) {
-    do_civil_war = TRUE;
+    /* Do a civil war only if there's an available unused nation. */
+    nations_iterate(pnation) {
+      if (is_nation_playable(pnation->index)
+         && !pnation->is_unavailable && !pnation->is_used) {
+       do_civil_war = TRUE;
+       break;
+      }
+    } nations_iterate_end;
   }
 
   /* 
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.397
diff -p -u -r1.397 plrhand.c
--- server/plrhand.c    6 Jul 2005 08:16:52 -0000       1.397
+++ server/plrhand.c    11 Jul 2005 23:30:11 -0000
@@ -64,7 +64,6 @@ static void package_player_info(struct p
                                 struct packet_player_info *packet,
                                 struct player *receiver,
                                 enum plr_info_level min_info_level);
-static Nation_type_id pick_available_nation(Nation_type_id *choices);
 static enum plr_info_level player_info_level(struct player *plr,
                                             struct player *receiver);
 
@@ -1296,64 +1295,93 @@ struct player *shuffled_player(int i)
   }
 }
 
+/**************************************************************************
+  Returns how much two nations looks good in the same game
+**************************************************************************/
+static int nations_match(struct nation_type* n1, struct nation_type* n2)
+{
+  int i, sum = 0;
+
+  for (i = 0; i < n1->num_groups; i++) {
+    if (nation_in_group(n2, n1->groups[i]->name)) {
+      sum += n1->groups[i]->match;
+    }
+  }
+  return sum;
+}
+
 /****************************************************************************
   This function return one of the nations available from the
   NO_NATION_SELECTED-terminated choices list. If no available nations in this
   file were found, return a random nation. If no nations are available, die.
+
+  choices may be NULL; if so it's ignored.
 ****************************************************************************/
-static Nation_type_id pick_available_nation(Nation_type_id *choices)
+Nation_type_id pick_available_nation(Nation_type_id *choices)
 {
-  int *nations_used, i, num_nations_avail = game.control.playable_nation_count;
-  int pick, looking_for, pref_nations_avail = 0; 
+  enum {
+    UNAVAILABLE, AVAILABLE, PREFERRED
+  } nations_used[game.control.nation_count], looking_for;
+  int match[game.control.nation_count], pick;
+  int num_nations_avail = 0, pref_nations_avail = 0;
 
   /* Values of nations_used: 
    * 0: not available
    * 1: available
    * 2: preferred choice */
-  nations_used = fc_calloc(game.control.playable_nation_count, sizeof(int));
+  nations_iterate(pnation) {
+    if (!is_nation_playable(pnation->index)
+       || pnation->is_used || pnation->is_unavailable) {
+      /* Nation is unplayable or already used: don't consider it. */
+      nations_used[pnation->index] = UNAVAILABLE;
+      match[pnation->index] = 0;
+      continue;
+    }
 
-  for (i = 0; i < game.control.playable_nation_count; i++) {
-    nations_used[i] = 1; /* Available (for now) */
-  }
+    nations_used[pnation->index] = AVAILABLE;
 
-  for (i = 0; choices[i] != NO_NATION_SELECTED; i++) {
-    pref_nations_avail++;
-    nations_used[choices[i]] = 2; /* Preferred */
-  }
+    match[pnation->index] = 1;
+    players_iterate(pplayer) {
+      if (pplayer->nation != NO_NATION_SELECTED) {
+       struct nation_type *pnation2 = get_nation_by_idx(pplayer->nation);
 
-  players_iterate(other_player) {
-    if (other_player->nation < game.control.playable_nation_count) {
-      if (nations_used[other_player->nation] == 2) {
-       pref_nations_avail--;
-      } 
-      nations_used[other_player->nation] = 0; /* Unavailable */
-      num_nations_avail--;
-    } 
-  } players_iterate_end;
+       match[pnation->index] += nations_match(pnation2, pnation) * 100;
+      }
+    } players_iterate_end;
+
+    num_nations_avail += match[pnation->index];
+  } nations_iterate_end;
+
+  for (; choices && *choices != NO_NATION_SELECTED; choices++) {
+    if (nations_used[*choices] == AVAILABLE) {
+      pref_nations_avail += match[*choices];
+      nations_used[*choices] = PREFERRED;
+    }
+  }
 
   assert(num_nations_avail > 0);
   assert(pref_nations_avail >= 0);
 
   if (pref_nations_avail == 0) {
     pick = myrand(num_nations_avail);
-    looking_for = 1; /* Use any available nation. */
+    looking_for = AVAILABLE; /* Use any available nation. */
   } else {
     pick = myrand(pref_nations_avail);
-    looking_for = 2; /* Use a preferred nation only. */
+    looking_for = PREFERRED; /* Use a preferred nation only. */
   }
 
-  for (i = 0; i < game.control.playable_nation_count; i++){ 
-    if (nations_used[i] == looking_for) {
-      pick--;
-      
+  nations_iterate(pnation) {
+    if (nations_used[pnation->index] == looking_for) {
+      pick -= match[pnation->index];
+
       if (pick < 0) {
-       break;
+       return pnation->index;
       }
     }
-  }
+  } nations_iterate_end;
 
-  free(nations_used);
-  return i;
+  assert(0);
+  return NO_NATION_SELECTED;
 }
 
 /****************************************************************************
Index: server/plrhand.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.h,v
retrieving revision 1.78
diff -p -u -r1.78 plrhand.h
--- server/plrhand.h    6 Jul 2005 08:16:52 -0000       1.78
+++ server/plrhand.h    11 Jul 2005 23:30:11 -0000
@@ -36,6 +36,8 @@ void kill_player(struct player *pplayer)
 void kill_dying_players(void);
 void update_revolution(struct player *pplayer);
 
+Nation_type_id pick_available_nation(Nation_type_id *choices);
+
 void check_player_government_rates(struct player *pplayer);
 void make_contact(struct player *pplayer1, struct player *pplayer2,
                  struct tile *ptile);
Index: server/srv_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
retrieving revision 1.277
diff -p -u -r1.277 srv_main.c
--- server/srv_main.c   11 Jul 2005 19:31:13 -0000      1.277
+++ server/srv_main.c   11 Jul 2005 23:30:14 -0000
@@ -1359,62 +1359,6 @@ void handle_player_ready(struct player *
   }
 }
 
-/**************************************************************************
-  Returns how much two nations looks good in the same game
-**************************************************************************/
-static int nations_match(struct nation_type* n1, struct nation_type* n2)
-{
-  int i;
-  int sum = 0;
-  for (i = 0; i < n1->num_groups; i++) {
-    if (nation_in_group(n2, n1->groups[i]->name)) {
-      sum += n1->groups[i]->match;
-    }
-  }
-  return sum;
-}
-
-/**************************************************************************
-  Select a random available nation.
-**************************************************************************/
-static Nation_type_id select_random_nation()
-{
-  Nation_type_id i, available[game.control.playable_nation_count];
-  int count = 0;
-  int V[game.control.playable_nation_count];
-  int sum = 0;
-  int x;
-  
-  /* Determine which nations are available. */
-  for (i = 0; i < game.control.playable_nation_count; i++) {
-    struct nation_type *nation = get_nation_by_idx(i);
-
-    if (!nation->is_unavailable && !nation->is_used) {
-      available[count] = i;
-      
-      /* Increase the probablity of selecting those which have higher
-       * values of nations_match() */
-      players_iterate(aplayer) {
-        if (aplayer->nation == NO_NATION_SELECTED) {
-         continue;
-       }
-       sum+= nations_match(get_nation_by_idx(aplayer->nation),
-                           get_nation_by_idx(i)) * 100;
-      } players_iterate_end;
-      sum++;
-      V[count] = sum;
-      count++;
-    }
-  }
-
-  /* Then pick one */  
-  x = myrand(sum);
-  for (i = 0; i < count; i++) {
-    if (V[i] >= x) break;
-  }
-  return available[i];
-}
-
 /****************************************************************************
   Fill or remove players to meet the given aifill.
 ****************************************************************************/
@@ -1504,7 +1448,6 @@ generate_ai_players() - Selects a nation
 **************************************************************************/
 static void generate_players(void)
 {
-  Nation_type_id nation;
   char player_name[MAX_LEN_NAME];
 
   /* Select nations for AI players generated with server
@@ -1519,34 +1462,33 @@ static void generate_players(void)
     }
 
     /* See if the player name matches a known leader name. */
-    for (nation = 0; nation < game.control.playable_nation_count; nation++) {
-      struct nation_type *n = get_nation_by_idx(nation);
-
-      if (check_nation_leader_name(nation, pplayer->name)
-         && !n->is_unavailable && !n->is_used) {
-       pplayer->nation = nation;
-       pplayer->city_style = get_nation_city_style(nation);
-       pplayer->is_male = get_nation_leader_sex(nation, pplayer->name);
+    nations_iterate(pnation) {
+      if (is_nation_playable(pnation->index)
+         && !pnation->is_unavailable && !pnation->is_used
+         && check_nation_leader_name(pnation->index, pplayer->name)) {
+       pplayer->nation = pnation->index;
+       pplayer->city_style = get_nation_city_style(pnation->index);
+       pplayer->is_male = get_nation_leader_sex(pnation->index,
+                                                pplayer->name);
        break;
       }
-    }
+    } nations_iterate_end;
     if (pplayer->nation != NO_NATION_SELECTED) {
       announce_player(pplayer);
       continue;
     }
 
-    nation = select_random_nation();
-    assert(nation != NO_NATION_SELECTED);
+    pplayer->nation = pick_available_nation(NULL);
+    assert(pplayer->nation != NO_NATION_SELECTED);
 
-    get_nation_by_idx(nation)->is_used = TRUE;
-    pplayer->nation = nation;
-    pplayer->city_style = get_nation_city_style(nation);
+    get_nation_by_idx(pplayer->nation)->is_used = TRUE;
+    pplayer->city_style = get_nation_city_style(pplayer->nation);
 
-    pick_random_player_name(nation, player_name);
+    pick_random_player_name(pplayer->nation, player_name);
     sz_strlcpy(pplayer->name, player_name);
 
-    if (check_nation_leader_name(nation, player_name)) {
-      pplayer->is_male = get_nation_leader_sex(nation, player_name);
+    if (check_nation_leader_name(pplayer->nation, player_name)) {
+      pplayer->is_male = get_nation_leader_sex(pplayer->nation, player_name);
     } else {
       pplayer->is_male = (myrand(2) == 1);
     }