[Freeciv-Dev] (PR#13473) remove playable_nation_count

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

This patch removes the playable_nation_count variable, and also the
restriction that barbarian/observer nations must be listed at the end.

Three new variables are added to the nation packet, struct, and ruleset:
is_playable, is_barbarian, and is_observer.  These tell whether regular,
barbarian, and observer players can use these nations.  I could have
used a 3-value enum but I don't see why these values shouldn't be
independent.

-jason

? class.diff
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.530
diff -p -u -r1.530 packhand.c
--- client/packhand.c   14 Jul 2005 06:59:27 -0000      1.530
+++ client/packhand.c   14 Jul 2005 17:42:14 -0000
@@ -2370,6 +2370,10 @@ void handle_ruleset_nation(struct packet
   }
   pl->city_style = p->city_style;
 
+  pl->is_playable = p->is_playable;
+  pl->is_observer = p->is_observer;
+  pl->is_barbarian = p->is_barbarian;
+
   memcpy(pl->init_techs, p->init_techs, sizeof(pl->init_techs));
   memcpy(pl->init_buildings, p->init_buildings, 
          sizeof(pl->init_buildings));
Index: common/nation.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.c,v
retrieving revision 1.54
diff -p -u -r1.54 nation.c
--- common/nation.c     11 Jul 2005 19:31:13 -0000      1.54
+++ common/nation.c     14 Jul 2005 17:42:16 -0000
@@ -115,9 +115,38 @@ const char *get_nation_name_orig(Nation_
 ****************************************************************************/
 bool is_nation_playable(Nation_type_id nation)
 {
-  /* Currently the nation index tells whether it's playable - barbarian
-   * and observer nations come at the end. */
-  return nation < game.control.playable_nation_count;
+  if (!bounds_check_nation_id(nation, LOG_FATAL, "is_nation_playable")) {
+    die("wrong nation %d", nation);
+  }
+  return nations[nation].is_playable;
+}
+
+/****************************************************************************
+  Return whether a nation is usable as an observer.  If true then observers
+  can use this nation.
+
+  This does not check whether a nation is "used" or "available".
+****************************************************************************/
+bool is_nation_observer(Nation_type_id nation)
+{
+  if (!bounds_check_nation_id(nation, LOG_FATAL, "is_nation_observer")) {
+    die("wrong nation %d", nation);
+  }
+  return nations[nation].is_observer;
+}
+
+/****************************************************************************
+  Return whether a nation is usable as a barbarian.  If true then barbarians
+  can use this nation.
+
+  This does not check whether a nation is "used" or "available".
+****************************************************************************/
+bool is_nation_barbarian(Nation_type_id nation)
+{
+  if (!bounds_check_nation_id(nation, LOG_FATAL, "is_nation_barbarian")) {
+    die("wrong nation %d", nation);
+  }
+  return nations[nation].is_barbarian;
 }
 
 /***************************************************************
Index: common/nation.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.h,v
retrieving revision 1.51
diff -p -u -r1.51 nation.h
--- common/nation.h     11 Jul 2005 19:31:13 -0000      1.51
+++ common/nation.h     14 Jul 2005 17:42:16 -0000
@@ -24,8 +24,6 @@
 /* Changing this value will break network compatibility. */
 #define NO_NATION_SELECTED (Nation_type_id)(-1)
 
-#define OBSERVER_NATION (game.control.nation_count - 2)
-
 /* 
  * Purpose of this constant is to catch invalid ruleset and network
  * data and to allow static allocation of the nation_info packet.
@@ -80,6 +78,8 @@ struct nation_type {
   struct city_name *city_names;                /* The default city names. */
   char *legend;                                /* may be empty */
 
+  bool is_playable, is_barbarian, is_observer;
+
   /* civilwar_nations is a NO_NATION_SELECTED-terminated list of index of
    * the nations that can fork from this one.  parent_nations is the inverse
    * of this array.  Server only. */
@@ -111,6 +111,8 @@ const char *get_nation_name(Nation_type_
 const char *get_nation_name_plural(Nation_type_id nation);
 const char *get_nation_name_orig(Nation_type_id nation);
 bool is_nation_playable(Nation_type_id nation);
+bool is_nation_observer(Nation_type_id nation);
+bool is_nation_barbarian(Nation_type_id nation);
 struct leader *get_nation_leaders(Nation_type_id nation, int *dim);
 Nation_type_id *get_nation_civilwar(Nation_type_id nation);
 bool get_nation_leader_sex(Nation_type_id nation, const char *name);
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.140
diff -p -u -r1.140 packets.def
--- common/packets.def  4 Jul 2005 18:42:27 -0000       1.140
+++ common/packets.def  14 Jul 2005 17:42:16 -0000
@@ -1163,6 +1163,8 @@ PACKET_RULESET_NATION=102;sc,lsend
   UINT8 leader_count;
   STRING leader_name[MAX_NUM_LEADERS:leader_count][MAX_LEN_NAME];
   BOOL leader_sex[MAX_NUM_LEADERS:leader_count];
+
+  BOOL is_playable, is_observer, is_barbarian;
   
   UINT8 group_count;
   STRING group_name[MAX_NUM_NATION_GROUPS:group_count][MAX_LEN_NAME];
@@ -1258,7 +1260,6 @@ PACKET_RULESET_CONTROL=106;sc,lsend
   UINT8 num_tech_types;
   UINT8 government_count;
   UINT8 nation_count;
-  UINT8 playable_nation_count;
   UINT8 styles_count;
   UINT8 terrain_count;
   UINT8 num_specialist_types;
Index: data/nation/barbarian.ruleset
===================================================================
RCS file: /home/freeciv/CVS/freeciv/data/nation/barbarian.ruleset,v
retrieving revision 1.10
diff -p -u -r1.10 barbarian.ruleset
--- data/nation/barbarian.ruleset       7 May 2005 14:03:52 -0000       1.10
+++ data/nation/barbarian.ruleset       14 Jul 2005 17:42:16 -0000
@@ -18,4 +18,8 @@ government = "Despotism"
 
 cities = "Nowhere"
 
+is_playable = 0
+is_observer = 0
+is_barbarian = 1
+
 ; nothing more needed for barbarians
Index: data/nation/observer.ruleset
===================================================================
RCS file: /home/freeciv/CVS/freeciv/data/nation/observer.ruleset,v
retrieving revision 1.3
diff -p -u -r1.3 observer.ruleset
--- data/nation/observer.ruleset        7 May 2005 14:03:53 -0000       1.3
+++ data/nation/observer.ruleset        14 Jul 2005 17:42:16 -0000
@@ -18,4 +18,8 @@ government = "Anarchy"
 
 cities = "Nowhere"
 
+is_playable = 0
+is_observer = 1
+is_barbarian = 0
+
 ; nothing more needed for observer
Index: server/barbarian.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/barbarian.c,v
retrieving revision 1.94
diff -p -u -r1.94 barbarian.c
--- server/barbarian.c  4 Jul 2005 17:48:38 -0000       1.94
+++ server/barbarian.c  14 Jul 2005 17:42:17 -0000
@@ -78,6 +78,30 @@ static bool is_sea_barbarian(struct play
   return (pplayer->ai.barbarian_type == SEA_BARBARIAN);
 }
 
+/****************************************************************************
+  Return an available barbarian nation.  This simply returns the first
+  available nation, or the first nation already in use by another barbarian
+  player.
+****************************************************************************/
+static Nation_type_id pick_barbarian_nation(void)
+{
+  nations_iterate(pnation) {
+    if (is_nation_barbarian(pnation->index) && !pnation->is_used) {
+      return pnation->index;
+    }
+  } nations_iterate_end;
+
+  players_iterate(pplayer) {
+    if (is_barbarian(pplayer)) {
+      assert(is_nation_barbarian(pplayer->nation));
+      return pplayer->nation;
+    }
+  } players_iterate_end;
+
+  assert(0);
+  return NO_NATION_SELECTED;
+}
+
 /**************************************************************************
   Creates the land/sea barbarian player and inits some stuff. If 
   barbarian player already exists, return player pointer. If barbarians 
@@ -89,6 +113,7 @@ static struct player *create_barbarian_p
 {
   int newplayer = game.info.nplayers;
   struct player *barbarians;
+  Nation_type_id nation = pick_barbarian_nation();
 
   players_iterate(barbarians) {
     if ((land && is_land_barbarian(barbarians))
@@ -97,8 +122,7 @@ static struct player *create_barbarian_p
         barbarians->economic.gold = 0;
         barbarians->is_alive = TRUE;
         barbarians->is_dying = FALSE;
-        pick_random_player_name(game.control.nation_count - 1,
-                               barbarians->name);
+        pick_random_player_name(nation, barbarians->name);
        sz_strlcpy(barbarians->username, ANON_USER_NAME);
         /* I need to make them to forget the map, I think */
        whole_map_iterate(ptile) {
@@ -120,8 +144,8 @@ static struct player *create_barbarian_p
 
   server_player_init(barbarians, TRUE, TRUE);
 
-  barbarians->nation = game.control.nation_count - 1;
-  pick_random_player_name(game.control.nation_count - 1, barbarians->name);
+  barbarians->nation = nation;
+  pick_random_player_name(nation, barbarians->name);
 
   game.info.nplayers++;
   game.info.nbarbarians++;
@@ -213,10 +237,12 @@ bool unleash_barbarians(struct tile *pti
     return FALSE;
   }
 
-  unit_cnt = 3 + myrand(4);
-
   barbarians = create_barbarian_player(TRUE);
+  if (!barbarians) {
+    return FALSE;
+  }
 
+  unit_cnt = 3 + myrand(4);
   for (i = 0; i < unit_cnt; i++) {
     unit = find_a_unit_type(L_BARBARIAN, L_BARBARIAN_TECH);
     (void) create_unit(barbarians, ptile, unit, 0, 0, -1);
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.398
diff -p -u -r1.398 plrhand.c
--- server/plrhand.c    13 Jul 2005 15:29:13 -0000      1.398
+++ server/plrhand.c    14 Jul 2005 17:42:17 -0000
@@ -1385,6 +1385,21 @@ Nation_type_id pick_available_nation(Nat
 }
 
 /****************************************************************************
+  Return an available observer nation.  This simply returns the first
+  such nation.  If no nation is available NO_NATION_SELECTED is returned.
+****************************************************************************/
+static Nation_type_id pick_observer_nation(void)
+{
+  nations_iterate(pnation) {
+    if (is_nation_observer(pnation->index) && !pnation->is_used) {
+      return pnation->index;
+    }
+  } nations_iterate_end;
+
+  return NO_NATION_SELECTED;
+}
+
+/****************************************************************************
   Create a player with is_observer = TRUE and return it.
   If a global observer has already been created, return that player.
   If there are no player slots available return NULL.
@@ -1394,6 +1409,7 @@ Nation_type_id pick_available_nation(Nat
 struct player *create_global_observer(void)
 {
   struct player *pplayer = NULL;
+  Nation_type_id nation;
 
   /* Check if a global observer already exists. If so, return it.  Note the
    * observer may exist at any position in the array. */
@@ -1412,6 +1428,13 @@ struct player *create_global_observer(vo
     return NULL;
   }
 
+  nation = pick_observer_nation();
+  if (nation == NO_NATION_SELECTED) {
+    notify_player(NULL, _("A global observer cannot be created: there's "
+                         "no observer nation in the ruleset."));
+    return NULL;
+  }
+
   /* alright, we can create an observer. go for it. */
   pplayer = &game.players[game.info.nplayers];
 
@@ -1438,7 +1461,7 @@ struct player *create_global_observer(vo
    *
    * FIXME: could we use map_is_empty here? */
   if (server_state == RUN_GAME_STATE || !game.info.is_new_game) {
-    pplayer->nation = OBSERVER_NATION;
+    pplayer->nation = nation;
     init_tech(pplayer, 0);
     map_know_and_see_all(pplayer);
   }
Index: server/ruleset.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/ruleset.c,v
retrieving revision 1.271
diff -p -u -r1.271 ruleset.c
--- server/ruleset.c    9 Jul 2005 17:46:09 -0000       1.271
+++ server/ruleset.c    14 Jul 2005 17:42:17 -0000
@@ -1848,16 +1848,6 @@ static void load_nation_names(struct sec
   (void) section_file_lookup(file, "datafile.description");    /* unused */
 
   sec = secfile_get_secnames_prefix(file, "nation", 
&game.control.nation_count);
-  game.control.playable_nation_count = game.control.nation_count - 2;
-  freelog(LOG_VERBOSE, "There are %d nations defined", 
-          game.control.playable_nation_count);
-
-  if (game.control.playable_nation_count < 0) {
-    freelog(LOG_FATAL,
-           "There must be at least one nation defined; number is %d",
-           game.control.playable_nation_count);
-    exit(EXIT_FAILURE);
-  }
   nations_alloc(game.control.nation_count);
 
   for (i = 0; i < game.control.nation_count; i++) {
@@ -2116,6 +2106,13 @@ static void load_ruleset_nations(struct 
     }
     free(leaders);
 
+    pl->is_playable = secfile_lookup_bool_default(file, TRUE,
+                                                 "%s.is_playable", sec[i]);
+    pl->is_observer = secfile_lookup_bool_default(file, FALSE,
+                                                 "%s.is_observer", sec[i]);
+    pl->is_barbarian = secfile_lookup_bool_default(file, FALSE,
+                                                 "%s.is_barbarian", sec[i]);
+
     /* Flags */
 
     sz_strlcpy(pl->flag_graphic_str,
@@ -2925,6 +2922,9 @@ static void send_ruleset_nations(struct 
       packet.leader_sex[i] = n->leaders[i].is_male;
     }
     packet.city_style = n->city_style;
+    packet.is_playable = n->is_playable;
+    packet.is_observer = n->is_observer;
+    packet.is_barbarian = n->is_barbarian;
     memcpy(packet.init_techs, n->init_techs, sizeof(packet.init_techs));
     memcpy(packet.init_buildings, n->init_buildings, 
            sizeof(packet.init_buildings));