[Freeciv-Dev] (PR#13425) distinguish playable nations

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

I started out by trying to change nations to use pointers instead of
indices.  The problem I ran into is that lots of nation iterations
should use nations_iterate but can't since that only iterates over
"playable" nations not all nations.  In fact, the playable-nation hack
has been a longstanding issue, so maybe it's time to fix it...

The playable-nations hack is that some nations are not "playable".  This
includes barbarian and observer nations.  These must be included at the
end of the ruleset so that game.control.nation_count and
game.control.playable_nation_count are two different values.  As you may
expect this causes problems because people never know which of these two
values to use.  This has been the cause of numerous bugs and ugly
workarounds.

This patch:

1.  Adds a new function is_nation_playable.
2.  Changes nations_iterate to iterate over all nations.
3.  Fixes the users of nations_iterate to use is_nation_playable.

As far as I can see this is the biggest design change needed to fix the
playable-nations hack.  After this we can change all sorts of other
iterators to use nations_iterate, we can add is_barbarian and
is_observer flags (or something to that effect) to the nation
struct/rulesets, and eventually we can remove
game.control.playable_nation_count entirely.

-jason

Index: common/nation.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.c,v
retrieving revision 1.53
diff -p -u -r1.53 nation.c
--- common/nation.c     8 Jul 2005 03:31:18 -0000       1.53
+++ common/nation.c     8 Jul 2005 21:51:45 -0000
@@ -107,6 +107,19 @@ const char *get_nation_name_orig(Nation_
   return nations[nation].name_orig;
 }
 
+/****************************************************************************
+  Return whether a nation is "playable"; i.e., whether a human player can
+  choose this nation.  Barbarian and observer nations are not playable.
+
+  This does not check whether a nation is "used" or "available".
+****************************************************************************/
+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;
+}
+
 /***************************************************************
 Returns pointer to the array of the nation leader names, and
 sets dim to number of leaders.
Index: common/nation.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.h,v
retrieving revision 1.50
diff -p -u -r1.50 nation.h
--- common/nation.h     8 Jul 2005 03:31:18 -0000       1.50
+++ common/nation.h     8 Jul 2005 21:51:45 -0000
@@ -110,6 +110,7 @@ Nation_type_id find_nation_by_name_orig(
 const char *get_nation_name(Nation_type_id nation);
 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);
 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);
@@ -130,12 +131,14 @@ bool nation_in_group(struct nation_type*
 bool can_conn_edit_players_nation(const struct connection *pconn,
                                  const struct player *pplayer);
 
+/* Iterate over nations.  This iterates over all nations, including
+ * unplayable ones (use is_nation_playable to filter if necessary). */
 #define nations_iterate(nation)                                                
    \
 {                                                                          \
   int NI_index;                                                                
    \
                                                                            \
   for (NI_index = 0;                                                       \
-       NI_index < game.control.playable_nation_count;                      \
+       NI_index < game.control.nation_count;                               \
        NI_index++) {                                                       \
     struct nation_type *nation = get_nation_by_idx(NI_index);
 
Index: server/srv_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
retrieving revision 1.276
diff -p -u -r1.276 srv_main.c
--- server/srv_main.c   29 Jun 2005 02:17:53 -0000      1.276
+++ server/srv_main.c   8 Jul 2005 21:51:46 -0000
@@ -1214,7 +1214,8 @@ void init_available_nations(void)
     }
   } else {
     nations_iterate(nation) {
-      nation->is_unavailable = FALSE;
+      /* We preemptively mark unplayable nations as unavailable. */
+      nation->is_unavailable = !is_nation_playable(nation->index);
     } nations_iterate_end;
   }
   nations_iterate(nation) {