[Freeciv-Dev] (PR#3930) remove MAX_NUM_NATIONS

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

[rfalke - Sat Apr  5 15:48:30 2003]:

> On Fri, Apr 04, 2003 at 03:09:22PM -0800, Jason Short wrote:
> > Mike Kaufman wrote:
> > > On Fri, Apr 04, 2003 at 12:27:10PM -0800, Jason Short wrote:
> > >
> > >>The attached patch increases MAX_NUM_NATIONS to 90, which requires
> a new
> > >>manditory capability.  This is a portion of the original PR#3589
> (which
> > >>was never updated with a capability).
> >
> > >>Ideally, MAX_NUM_NATIONS should be removed so that the server and
> client
> > >>do not need to have identical values for it.  This would also mean
> less
> > >>wasted space in most cases.  Unfortunately it's not trivial to do
> this -
> > >>but any patches to help toward that goal would be welcome.
> >
> >
> > > hell yes it should be removed. In fact, looking at the code, there
> seems to
> > > be no reason whatsoever for MAX_NUM_NATIONS to exist at all. Any
> place
> > > where should be used should use game.nation_count and dynamically
> allocate
> > > the arrays. MAX_NUM_NATIONS seems to be left over from Raimar's
> 64k purge a
> > > while back. My only qualm is the possibility of having to send a
> couple
> > > thousand nations to the client :)
> >
> > Yes, exactly.  The only problem is that it may take some work to get
> > there - the code is pretty spread out and embedded in various parts
> of
> > thee code, and it's not trivial (for me) to tell where the
> allocations
> > need to go.
> 
> It isn't _this_ hard. Lets see:
> 
> ./client/packhand.c:2262:  if(p->id < 0 || p->id >= game.nation_count
> || p->id >= MAX_NUM_NATIONS) {
> ./server/ruleset.c:1831:  } else if (game.playable_nation_count >=
> MAX_NUM_NATIONS) {
> ./server/ruleset.c:1834:            MAX_NUM_NATIONS - 1,
> game.playable_nation_count);
> ./server/ruleset.c:1835:    game.playable_nation_count =
> MAX_NUM_NATIONS - 1;
> 
> Safeguards. Can be removed.
> 
> ./client/gui-gtk-2.0/plrdlg.c:57:static GdkPixbuf
> *flags[MAX_NUM_NATIONS];
> 
> game.nation_count
> 
> ./server/srv_main.c:1113:    if (pplayer->nation == MAX_NUM_NATIONS) {
> 
> Should be NO_NATION_SELECTED.
> 
> ./common/map.h:182:  struct map_position
> start_positions[MAX_NUM_NATIONS];
> 
> This is only non-trival one. Alloc space for game.nplayers in
> create_start_positions and also alloc space in map_startpos_load. Not
> this hard.

The attached patch removes MAX_NUM_NATIONS.  It has been tested only
minimally; I'm not all that sure of correctness.  Raimar, can you look
at it?

jason

Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.301
diff -u -r1.301 packhand.c
--- client/packhand.c   2003/04/17 20:06:35     1.301
+++ client/packhand.c   2003/04/25 18:47:31
@@ -2264,7 +2264,7 @@
   int i;
   struct nation_type *pl;
 
-  if(p->id < 0 || p->id >= game.nation_count || p->id >= MAX_NUM_NATIONS) {
+  if (p->id < 0 || p->id >= game.nation_count) {
     freelog(LOG_ERROR, "Received bad nation id %d in handle_ruleset_nation()",
            p->id);
     return;
Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.132
diff -u -r1.132 capstr.c
--- common/capstr.c     2003/04/22 20:50:56     1.132
+++ common/capstr.c     2003/04/25 18:47:31
@@ -77,7 +77,8 @@
 #define CAPABILITY "+1.14.0 conn_info +occupied team tech_impr_gfx " \
                    "city_struct_minor_cleanup obsolete_last class_legend " \
                    "+impr_req +waste +fastfocus +continent +small_dipl " \
-                   "+no_nation_selected +diplomacy +no_extra_tiles"
+                   "+no_nation_selected +diplomacy +no_extra_tiles " \
+                   "+unlimited_nations"
 /* "+1.14.0" is protocol for 1.14.0 release.
  *
  * "conn_info" is sending the conn_id field. To preserve compatability
@@ -120,6 +121,9 @@
  *
  * "no_extra_tiles" means that the extra, unknown (NODRAW) tiles will not be
  * sent to clients (previously used to help with drawing the terrain).
+ *
+ * "unlimited_nations" means that the MAX_NUM_NATIONS limit has been removed,
+ * allowing easy adding of more nations.
  */
 
 void init_our_capability(void)
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.145
diff -u -r1.145 map.h
--- common/map.h        2003/04/22 20:50:56     1.145
+++ common/map.h        2003/04/25 18:47:31
@@ -176,7 +176,7 @@
   struct tile *tiles;
 
   /* Only used by server. */
-  struct map_position start_positions[MAX_NUM_NATIONS];
+  struct map_position *start_positions;
 };
 
 bool map_is_empty(void);
Index: common/nation.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.h,v
retrieving revision 1.23
diff -u -r1.23 nation.h
--- common/nation.h     2003/04/01 17:19:49     1.23
+++ common/nation.h     2003/04/25 18:47:31
@@ -18,8 +18,7 @@
 
 #define MAX_NUM_TECH_GOALS 10
 
-/* Changing either of these values will break network compatibility. */
-#define MAX_NUM_NATIONS  63
+/* Changing this value will break network compatibility. */
 #define NO_NATION_SELECTED (Nation_Type_id)(-1)
 
 /* 
Index: server/mapgen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
retrieving revision 1.110
diff -u -r1.110 mapgen.c
--- server/mapgen.c     2003/02/20 09:45:22     1.110
+++ server/mapgen.c     2003/04/25 18:47:34
@@ -1102,6 +1102,9 @@
   }
   assert(game.nplayers<=nr+sum);
 
+  map.start_positions = fc_realloc(map.start_positions,
+                                  game.nplayers
+                                  * sizeof(*map.start_positions));
   while (nr<game.nplayers) {
     rand_map_pos(&x, &y);
     if (islands[(int)map_get_continent(x, y)].starters != 0) {
Index: server/ruleset.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/ruleset.c,v
retrieving revision 1.140
diff -u -r1.140 ruleset.c
--- server/ruleset.c    2003/04/04 15:47:50     1.140
+++ server/ruleset.c    2003/04/25 18:47:35
@@ -1828,11 +1828,6 @@
            "There must be at least one nation defined; number is %d",
            game.playable_nation_count);
     exit(EXIT_FAILURE);
-  } else if (game.playable_nation_count >= MAX_NUM_NATIONS) {
-    freelog(LOG_ERROR,
-           "There are too many nations; only using %d of %d available.",
-           MAX_NUM_NATIONS - 1, game.playable_nation_count);
-    game.playable_nation_count = MAX_NUM_NATIONS - 1;
   }
   nations_alloc(game.nation_count);
 
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.116
diff -u -r1.116 savegame.c
--- server/savegame.c   2003/04/22 20:50:56     1.116
+++ server/savegame.c   2003/04/25 18:47:36
@@ -276,7 +276,12 @@
 
   map.fixed_start_positions = secfile_lookup_bool_default(file, FALSE, 
"map.fixed_start_positions");
 
-  while( (pos = secfile_lookup_int_default(file, -1, "map.r%dsx", i)) != -1) {
+  map.start_positions = fc_realloc(map.start_positions,
+                                  game.max_players
+                                  * sizeof(*map.start_positions));
+  while (i < game.max_players
+        && (pos = secfile_lookup_int_default(file, -1,
+                                             "map.r%dsx", i)) != -1) {
     map.start_positions[i].x = pos;
     map.start_positions[i].y = secfile_lookup_int(file, "map.r%dsy", i);
     i++;