[Freeciv-Dev] Re: (PR#8749) Scenario fixes

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8749 >

I'd already started writing my patch, so I continued writing it.

This patch makes all the necessary savegame and start-pos-assigning 
changes.  However it does not restrict the nations to those in the 
scenario.  I agree we should do this, however we can only do it if all 
start positions in the scenario have nations associated.  It should be 
left for a later patch IMO.

The algorithm is simple:

- First assign nations to start positions that have a listed nation.

- Second randomly assign remaining players to remaining start positions.

The idea is to cover all possibilities without any special cases:

* Normal games have no nations given for any start positions.

* Old scenarios use the indexed method to associate nations with start 
positions.  This is useless since the nations have been reordered.  So 
it's treated the same as the first case.

* New scenarios will have nations listed for start positions.  However 
in most cases not all nations will be listed.  So the remaining players 
must be assigned to the remaining nations.  This may be improved on 
later by limiting the nations to just those that are listed.

* Broken scenarios may list nations for some start positions only. 
Although broken, this will work.

* "Fair" scenarios may intentionally have start positions with no 
nations associated with them.  This could be used for tournament-style 
play or for a scenario generated by an external mapgen.

jason

Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.182
diff -u -r1.182 map.h
--- common/map.h        5 May 2004 21:58:11 -0000       1.182
+++ common/map.h        15 May 2004 23:55:43 -0000
@@ -140,7 +140,10 @@
   struct tile *tiles;
 
   /* Only used by server. */
-  struct map_position *start_positions;        /* allocated at runtime */
+  struct start_position {
+    int x, y;
+    Nation_Type_id nation; /* May be NO_NATION_SELECTED. */
+  } *start_positions;  /* allocated at runtime */
 };
 
 enum topo_flag {
Index: common/nation.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.c,v
retrieving revision 1.37
diff -u -r1.37 nation.c
--- common/nation.c     15 Sep 2003 16:51:07 -0000      1.37
+++ common/nation.c     15 May 2004 23:55:43 -0000
@@ -68,7 +68,7 @@
      if(mystrcasecmp(name, get_nation_name (i)) == 0)
        return i;
 
-  return -1;
+  return NO_NATION_SELECTED;
 }
 
 /***************************************************************
Index: server/gamehand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gamehand.c,v
retrieving revision 1.132
diff -u -r1.132 gamehand.c
--- server/gamehand.c   14 May 2004 11:04:22 -0000      1.132
+++ server/gamehand.c   15 May 2004 23:55:44 -0000
@@ -91,107 +91,63 @@
 }
 
 /****************************************************************************
-  Swap two map positions.
-****************************************************************************/
-static void swap_map_positions(struct map_position *a,
-                              struct map_position *b)
-{
-  struct map_position tmp;
-
-  tmp = *a;
-  *a = *b;
-  *b = tmp;
-}
-
-/****************************************************************************
-  Get the start position for the given player.
-****************************************************************************/
-static struct map_position get_start_position(struct player *pplayer,
-                                             int *start_pos)
-{
-  if (map.fixed_start_positions) {
-    return map.start_positions[start_pos[pplayer->player_no]];
-  } else {
-    return map.start_positions[pplayer->player_no];
-  }
-}
-
-/****************************************************************************
   Initialize a new game: place the players' units onto the map, etc.
 ****************************************************************************/
 void init_new_game(void)
 {
-  Nation_Type_id start_pos[MAX_NUM_PLAYERS];
+#define NO_START_POS -1
+  int start_pos[game.nplayers];
+  bool pos_used[map.num_start_positions];
+  int i, num_used = 0;
+
   init_game_id();
 
   /* Shuffle starting positions around so that they match up with the
    * desired players. */
-  if (!map.fixed_start_positions) {
-    /* With non-fixed starting positions, just randomize the order.  This
-     * avoids giving an advantage to lower-numbered players. */
-    assert(game.nplayers == map.num_start_positions);
-    players_iterate(pplayer) {
-      swap_map_positions(&map.start_positions[pplayer->player_no],
-                        &map.start_positions[myrand(game.nplayers)]);
-    } players_iterate_end;
-  } else {
-    /* In a scenario, choose starting positions by nation.  If there are too
-     * few starts for number of nations, assign to nations with specific
-     * starts first, then assign the rest to random from remainder.  (It
-     * would be better to label start positions by nation etc, but this will
-     * do for now.)
-     *
-     * NOTE: map.num_start_positions may be very high.
-     *
-     * FIXME: this method is broken since it assumes nations have a constant
-     * id, which is generally not true. */
-    const int npos = map.num_start_positions;
-    int nrem = npos, player_no;
-    bool *pos_used = fc_calloc(map.num_start_positions, sizeof(*pos_used));
-
-    /* Match nation 0 to starting position 0, and so on.  This needs an
-     * explicit for loop to guarantee the proper ordering. */
-    assert(game.nplayers <= map.num_start_positions);
-    for (player_no = 0; player_no < game.nplayers; player_no++) {
-      Nation_Type_id nation = game.players[player_no].nation;
-
-      if (nation < npos) {
-       start_pos[player_no] = nation;
-       pos_used[nation] = TRUE;
-       nrem--;
-      } else {
-       start_pos[player_no] = NO_NATION_SELECTED;
+
+  /* First set up some data fields. */
+  for (i = 0; i < map.num_start_positions; i++) {
+    pos_used[i] = FALSE;
+  }
+  players_iterate(pplayer) {
+    start_pos[pplayer->player_no] = NO_START_POS;
+  } players_iterate_end;
+
+  /* Second, assign a nation to a start position for that nation. */
+  players_iterate(pplayer) {
+    for (i = 0; i < map.num_start_positions; i++) {
+      assert(pplayer->nation != NO_NATION_SELECTED);
+      if (pplayer->nation == map.start_positions[i].nation) {
+       start_pos[pplayer->player_no] = i;
+       num_used++;
       }
     }
+  } players_iterate_end;
 
-    /* Now randomize the unchosen nations. */
-    players_iterate(pplayer) {
-      if (start_pos[pplayer->player_no] == NO_NATION_SELECTED) {
-       int j, k;
-
-       assert(nrem > 0);
-       k = myrand(nrem);
-       for (j = 0; j < npos; j++) {
-         if (!pos_used[j] && (0 == k--)) {
-           start_pos[pplayer->player_no] = j;
-           pos_used[j] = TRUE;
-           nrem--;
+  /* Third, assign players randomly to the remaining start positions. */
+  players_iterate(pplayer) {
+    if (start_pos[pplayer->player_no] == NO_START_POS) {
+      int which = myrand(map.num_start_positions - num_used);
+
+      for (i = 0; i < map.num_start_positions; i++) {
+       if (!pos_used[i]) {
+         if (which == 0) {
+           start_pos[pplayer->player_no] = i;
+           pos_used[i] = 0;
+           num_used++;
            break;
          }
+         which--;
        }
-       assert(start_pos[pplayer->player_no] != NO_NATION_SELECTED);
       }
-    } players_iterate_end;
-
-    /* The starting positions are now stored in the start_pos array, and
-     * may be accessed via the get_start_position function. */
-
-    free(pos_used);
-  }
+    }
+    assert(start_pos[pplayer->player_no] != NO_START_POS);
+  } players_iterate_end;
 
   /* Loop over all players, creating their initial units... */
   players_iterate(pplayer) {
-    struct map_position pos = get_start_position(pplayer, start_pos);
+    struct start_position pos
+      = map.start_positions[start_pos[pplayer->player_no]];
 
     /* Place the first unit. */
     assert(game.settlers > 0);
@@ -201,7 +157,8 @@
   /* Place all other units. */
   players_iterate(pplayer) {
     int i, x, y;
-    struct map_position p = get_start_position(pplayer, start_pos);
+    struct start_position p
+      = map.start_positions[start_pos[pplayer->player_no]];
 
     for (i = 1; i < (game.settlers + game.explorer); i++) {
       do {
Index: server/mapgen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
retrieving revision 1.132
diff -u -r1.132 mapgen.c
--- server/mapgen.c     20 Apr 2004 20:10:38 -0000      1.132
+++ server/mapgen.c     15 May 2004 23:55:44 -0000
@@ -1252,6 +1252,7 @@
       islands[(int)map_get_continent(x, y)].starters--;
       map.start_positions[data.count].x = x;
       map.start_positions[data.count].y = y;
+      map.start_positions[data.count].nation = NO_NATION_SELECTED;
       freelog(LOG_DEBUG, "Adding %d,%d as starting position %d.",
              x, y, data.count);
       data.count++;
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.149
diff -u -r1.149 savegame.c
--- server/savegame.c   26 Mar 2004 17:31:55 -0000      1.149
+++ server/savegame.c   15 May 2004 23:55:45 -0000
@@ -302,8 +302,22 @@
   while (i < game.max_players
         && (pos = secfile_lookup_int_default(file, -1,
                                              "map.r%dsx", i)) != -1) {
+    char *nation = secfile_lookup_str_default(file, NULL, "map.r%dsn", i);
+
     map.start_positions[i].x = pos;
     map.start_positions[i].y = secfile_lookup_int(file, "map.r%dsy", i);
+
+    if (nation) {
+      /* This will fall back to NO_NATION_SELECTED if the string doesn't
+       * match any nation. */
+      map.start_positions[i].nation = find_nation_by_name(nation);
+    } else {
+      /* Old-style nation ordering is useless to us because the nations
+       * have been reordered.  Just ignore it and order the nations
+       * randomly. */
+      map.start_positions[i].nation = NO_NATION_SELECTED;
+    }
+
     i++;
   }