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

["Per Inge Mathisen" <per@xxxxxxxxxxx>]

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

Adds a new field to the fixed start positions in savegames which holds the
nation in text. If present, this is used instead of fixed indexing into
the nations list, which is a very flawed way to indicate a nation, since
we add new ones every now and then, throwing the index off track.

Also simplify and sanitize some strange code.

Finally forbid client selection of nations not in the fixed starting
positions list when we use scenarios with such lists.

Mike, I think you want a look at this, since it impacts on civworld.

  - Per

Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.165
diff -u -r1.165 capstr.c
--- common/capstr.c     11 May 2004 17:18:19 -0000      1.165
+++ common/capstr.c     15 May 2004 22:44:52 -0000
@@ -77,7 +77,7 @@
 #define CAPABILITY "+1.14.delta +last_turns_shield_surplus veteran +orders " \
                    "+starter +union +iso_maps +orders2client " \
                    "+change_production +tilespec1 +no_earth +trans " \
-                   "+want_hack invasions bombard +killstack2"
+                   "+want_hack invasions bombard +killstack2 +scenario"
 
 /* "+1.14.delta" is the new delta protocol for 1.14.0-dev.
  *
@@ -115,6 +115,8 @@
  * it's a boolean.
  *
  * "bombard" means units support the bombard ability.
+ *
+ * "scenario" means better support for scenarios.
  */
 
 void init_our_capability(void)
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 22:44:52 -0000
@@ -115,6 +115,11 @@
   char *helptext;
 };
 
+struct start_position {
+  int x, y;
+  Nation_Type_id nation;
+};
+
 struct civ_map { 
   int topology_id;
   int xsize, ysize; /* native dimensions */
@@ -140,7 +145,7 @@
   struct tile *tiles;
 
   /* Only used by server. */
-  struct map_position *start_positions;        /* allocated at runtime */
+  struct start_position start_positions[MAX_NUM_PLAYERS];
 };
 
 enum topo_flag {
Index: common/packets.def
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets.def,v
retrieving revision 1.22
diff -u -r1.22 packets.def
--- common/packets.def  11 May 2004 17:18:19 -0000      1.22
+++ common/packets.def  15 May 2004 22:44:52 -0000
@@ -266,7 +266,7 @@
 
 PACKET_NATIONS_SELECTED_INFO=9;sc,lsend
   UINT8 num_nations_used;
-  NATION nations_used[MAX_NUM_PLAYERS:num_nations_used];
+  NATION nations_used[MAX_NUM_ITEMS:num_nations_used];
 end
 
 PACKET_NATION_SELECT_REQ=10;cs,dsend
Index: common/packets_gen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets_gen.c,v
retrieving revision 1.25
diff -u -r1.25 packets_gen.c
--- common/packets_gen.c        11 May 2004 17:18:19 -0000      1.25
+++ common/packets_gen.c        15 May 2004 22:44:55 -0000
@@ -1542,9 +1542,9 @@
     {
       int i;
     
-      if(real_packet->num_nations_used > MAX_NUM_PLAYERS) {
+      if(real_packet->num_nations_used > MAX_NUM_ITEMS) {
         freelog(LOG_ERROR, "packets_gen.c: WARNING: truncation array");
-        real_packet->num_nations_used = MAX_NUM_PLAYERS;
+        real_packet->num_nations_used = MAX_NUM_ITEMS;
       }
       for (i = 0; i < real_packet->num_nations_used; i++) {
         dio_get_uint16(&din, (int *) &real_packet->nations_used[i]);
Index: common/packets_gen.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/packets_gen.h,v
retrieving revision 1.20
diff -u -r1.20 packets_gen.h
--- common/packets_gen.h        11 May 2004 17:18:20 -0000      1.20
+++ common/packets_gen.h        15 May 2004 22:44:55 -0000
@@ -54,7 +54,7 @@
 
 struct packet_nations_selected_info {
   int num_nations_used;
-  Nation_Type_id nations_used[MAX_NUM_PLAYERS];
+  Nation_Type_id nations_used[MAX_NUM_ITEMS];
 };
 
 struct packet_nation_select_req {
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 22:44:55 -0000
@@ -91,37 +91,12 @@
 }
 
 /****************************************************************************
-  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];
+  struct start_position starts[MAX_NUM_PLAYERS];
+
   init_game_id();
 
   /* Shuffle starting positions around so that they match up with the
@@ -131,84 +106,54 @@
      * 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)]);
+      int who = myrand(game.nplayers);
+      struct start_position swap = map.start_positions[who];
+
+      map.start_positions[who] = map.start_positions[pplayer->player_no];
+      map.start_positions[pplayer->player_no] = swap;
+    } players_iterate_end;
+    players_iterate(pplayer) {
+      starts[pplayer->player_no] = map.start_positions[pplayer->player_no];
     } 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;
-      }
-    }
-
-    /* Now randomize the unchosen nations. */
+    /* Choose starting positions by nation. We never have more players
+     * than starting positions here, any excesses have already been
+     * culled in stdinhand.c. We have also prohibited any nation
+     * selections other than those available from our start positions.
+     */
     players_iterate(pplayer) {
-      if (start_pos[pplayer->player_no] == NO_NATION_SELECTED) {
-       int j, k;
+      int i;
 
-       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--;
-           break;
-         }
-       }
-       assert(start_pos[pplayer->player_no] != NO_NATION_SELECTED);
+      for (i = 0; i < map.num_start_positions; i++) {
+        if (map.start_positions[i].nation == pplayer->nation) {
+          starts[pplayer->player_no].x = map.start_positions[i].x;
+          starts[pplayer->player_no].y = map.start_positions[i].y;
+          starts[pplayer->player_no].nation = pplayer->nation;
+        }
       }
     } 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);
   }
 
   /* Loop over all players, creating their initial units... */
   players_iterate(pplayer) {
-    struct map_position pos = get_start_position(pplayer, start_pos);
+    int plrno = pplayer->player_no;
 
     /* Place the first unit. */
     assert(game.settlers > 0);
-    place_starting_unit(pos.x, pos.y, pplayer, F_CITIES);
+    place_starting_unit(starts[plrno].x, starts[plrno].y, pplayer, F_CITIES);
   } players_iterate_end;
 
   /* Place all other units. */
   players_iterate(pplayer) {
     int i, x, y;
-    struct map_position p = get_start_position(pplayer, start_pos);
+    int px = starts[pplayer->player_no].x, py = starts[pplayer->player_no].y;
 
     for (i = 1; i < (game.settlers + game.explorer); i++) {
       do {
-       x = p.x + myrand(2 * game.dispersion + 1) - game.dispersion;
-       y = p.y + myrand(2 * game.dispersion + 1) - game.dispersion;
+       x = px + myrand(2 * game.dispersion + 1) - game.dispersion;
+       y = py + myrand(2 * game.dispersion + 1) - game.dispersion;
       } while (!(normalize_map_pos(&x, &y)
-                && map_get_continent(p.x, p.y) == map_get_continent(x, y)
+                && map_get_continent(px, py) == map_get_continent(x, y)
                 && !is_ocean(map_get_terrain(x, y))
                 && !is_non_allied_unit_tile(map_get_tile(x, y),
                                             pplayer)));
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 22:44:56 -0000
@@ -1244,9 +1244,6 @@
   }
   assert(game.nplayers <= data.count + sum);
 
-  map.start_positions = fc_realloc(map.start_positions,
-                                  game.nplayers
-                                  * sizeof(*map.start_positions));
   while (data.count < game.nplayers) {
     if (rand_map_pos_filtered(&x, &y, &data, is_valid_start_pos)) {
       islands[(int)map_get_continent(x, y)].starters--;
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.150
diff -u -r1.150 savegame.c
--- server/savegame.c   15 May 2004 22:41:37 -0000      1.150
+++ server/savegame.c   15 May 2004 22:44:56 -0000
@@ -284,26 +284,27 @@
 }
 
 /***************************************************************
-load starting positions for the players from a savegame file
-Now we don't know how many start positions there are nor how many
-should be because rulesets are loaded later. So try to load as
-many as they are; there should be at least enough for every
-player.  This could be changed/improved in future.
+  Load starting positions for the players from a savegame file.
 ***************************************************************/
 static void map_startpos_load(struct section_file *file)
 {
-  int i=0, pos;
+  int i = 0, pos;
 
   map.fixed_start_positions = secfile_lookup_bool_default(file, FALSE, 
"map.fixed_start_positions");
 
-  map.start_positions = fc_realloc(map.start_positions,
-                                  game.max_players
-                                  * sizeof(*map.start_positions));
-  while (i < game.max_players
+  while (i < MAX_NUM_PLAYERS
         && (pos = secfile_lookup_int_default(file, -1,
                                              "map.r%dsx", i)) != -1) {
+    char *str = NULL;
+
     map.start_positions[i].x = pos;
     map.start_positions[i].y = secfile_lookup_int(file, "map.r%dsy", i);
+    str = secfile_lookup_str_default(file, NULL, "map.r%dsnation", i);
+    if (str != NULL) {
+      map.start_positions[i].nation = find_nation_by_name_orig(str);
+    } else {
+      map.start_positions[i].nation = i; /* and hope for the best... */
+    }
     i++;
   }
 
Index: server/srv_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
retrieving revision 1.160
diff -u -r1.160 srv_main.c
--- server/srv_main.c   2 May 2004 14:47:12 -0000       1.160
+++ server/srv_main.c   15 May 2004 22:44:56 -0000
@@ -983,6 +983,8 @@
   /* tell the other players, that the nation is now unavailable */
   nation_used_count = 0;
 
+  mark_nation_as_used(nation_no);
+
   players_iterate(other_player) {
     if (other_player->nation == NO_NATION_SELECTED) {
       send_select_nation(other_player);
@@ -991,8 +993,6 @@
     }
   } players_iterate_end;
 
-  mark_nation_as_used(nation_no);
-
   /* if there's no nation left, reject remaining players, sorry */
   if( nation_used_count == game.playable_nation_count ) {   /* barb */
     players_iterate(other_player) {
@@ -1013,16 +1013,16 @@
 static void send_select_nation(struct player *pplayer)
 {
   struct packet_nations_selected_info packet;
+  int i;
 
   packet.num_nations_used = 0;
 
-  players_iterate(other_player) {
-    if (other_player->nation == NO_NATION_SELECTED) {
-      continue;
+  for (i = 0; i < game.playable_nation_count; i++) {
+    if (nations_used[i] == -1) {
+      packet.nations_used[packet.num_nations_used] = i;
+      packet.num_nations_used++;
     }
-    packet.nations_used[packet.num_nations_used] = other_player->nation;
-    packet.num_nations_used++;
-  } players_iterate_end;
+  }
 
   lsend_packet_nations_selected_info(&pplayer->connections, &packet);
 }
@@ -1131,7 +1131,7 @@
       continue;
     }
 
-    if (num_nations_avail == 0) {
+    if (num_nations_avail == 0) { freelog(LOG_NORMAL, "at %d", i);
       freelog(LOG_NORMAL,
              _("Ran out of nations.  AI controlled player %s not created."),
              pplayer->name );
@@ -1558,6 +1558,27 @@
     nations_used[i] = i;
   }
 
+  /* Mark any nations for which we do not have starting positions
+   * as used. */
+  if (map.fixed_start_positions) {
+    int idx;
+
+    assert(game.nplayers <= map.num_start_positions);
+    for (idx = 0; idx < game.playable_nation_count; idx++) {
+      bool avail = FALSE;
+      int sp;
+
+      for (sp = 0; sp < map.num_start_positions; sp++) {
+        if (map.start_positions[idx].nation == idx) {
+          avail = TRUE;
+        }
+      }
+      if (!avail) {
+        mark_nation_as_used(idx);
+      }
+    }
+  }
+
   if (game.auto_ai_toggle) {
     players_iterate(pplayer) {
       if (!pplayer->is_connected && !pplayer->ai.control) {