Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2002:
[Freeciv-Dev] Re: new natural names patch (PR#1127)
Home

[Freeciv-Dev] Re: new natural names patch (PR#1127)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: new natural names patch (PR#1127)
From: jdorje@xxxxxxxxxxxxxxxxxxxxx
Date: Wed, 9 Jan 2002 17:56:37 -0800 (PST)

Raimar Falke wrote:

> On Sun, Jan 06, 2002 at 06:35:15PM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>
>>Raimar Falke wrote:
>>
>>
>>>On Sat, Dec 22, 2001 at 12:31:31PM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>>>
>>>
>>>>diff -u -r1.7 nation.h
>>>>--- common/nation.h     2001/12/06 11:59:03     1.7
>>>>+++ common/nation.h     2001/12/22 20:22:14
>>>>@@ -27,6 +27,28 @@
>>>>enum advisor_type {ADV_ISLAND, ADV_MILITARY, ADV_TRADE, ADV_SCIENCE, ADV_FOREIGN,
>>>>                   ADV_ATTITUDE, ADV_DOMESTIC, ADV_LAST};
>>>>
>>>>+/*
>>>>+ * The city_name structure holds information about a default choice for
>>>>+ * the city name.  The "name" field is, of course, just the name for
>>>>+ * the city.  The "value" is the priority rating of this name - lower
>>>>+ * priority cities will show up sooner. The "river" and "terrain" fields >>>>+ * are entries recording whether the terrain is present near the city - >>>>+ * we give higher priority to cities which have matching terrain. In the >>>>+ * case of a river we only care if the city is _on_ the river, for other
>>>>+ * terrain features we give the bonus if the city is close to the
>>>>+ * terrain. Both of these entries may hold a value of 0 (no preference),
>>>>+ * 1 (city likes the terrain), or -1 (city doesn't like the terrain).
>>>>
>>>>
>>>Either make the three values fix via enum or DEFINE or allow a wider
>>>range (2 stands for really like it and 10 for really really like it).

These values represent what is in the ruleset, so they must stick to
(-1, 0, 1) (or at least 3 values representing that).  I don't really
think these need or should have an explicit #define, since they're just
unit values and directly depend on that fact (see the multiplication in the calculation). But I'll add it if you really want it...


>>>>+/****************************************************************
>>>>+Searches through a city name list (a struct city_name array)
>>>>+to pick the best available city name, and returns a pointer to
>>>>+it.  The function uses its own internal algorithm to prioritize
>>>>+the city names; this algorithm need not always return the same
>>>>+name when given the same list.  If the list has no valid
>>>>+entries in it, NULL will be returned.
>>>>+*****************************************************************/
>>>>+static char *search_for_city_name(int x, int y, struct city_name *city_names)
>>>>+{
>>>>+  struct city_name *choice;
>>>>+  int best_value = -1, goodness;
>>>>+  char* best_name = NULL;
>>>>+  for (choice = city_names; choice->name; choice++) {
>>>>+    if (!game_find_city_by_name(choice->name)) {
>>>>+      /* Lower values are better. */
>>>>+      int value = choice->value, type;
>>>>+
>>>>+      /* We do a little randomizing of our own. */
>>>>+      value *= 10 + myrand(5);
>>>>
>>>>
>>>Why 10? Why 5? E.g. what range does value have?
>>>
>>The whole formula thing is pretty fuzzy.
>>
>>- City priorities are initialized to 10, 11, 12, ..., n+9
>>- Value is multiplied by 10.  This is just to avoid rounding problems;
>>similar to what freeciv does in combat calculations.
>>- myrand(5) is added.  This is just a very small random factor.
>>- Any additional terrain factors are multiplied/divided out.  I though a
>>matching terrain should be about twice as good as a non-matching
>>terrain, which is where the 1.4 comes from.
>>
>>Unless someone has a better idea for the formula, the only thing I think
>>needs changing is that I'll initialize the priorities to 1,2,3,...,n+10
>>and then add on 9 or 10 in the calculation.  This will move everything
>>into the calculation function.  I may also separate the calculation out
>>into it's own function, and I will in any case add a comment about the
>>fuzziness.  Later we can fine-tune the calculation, but it really
>>doesn't matter too much.
>>
>
> You don't have to explain it to me. Explain it to the future
> programmers who read the code.

OK.


>>>>+    memset(&city_names[j].terrain[0], 0,
>>>>+          T_COUNT * sizeof(city_names[j].terrain[0]));
>>>>
>>>>
>>>If somewhere is a
>>>  #define CITY_NAME_DOESNT_CARE 0
>>>is used you can't use the memset.
>>>
>>I don't quite understand.
>
> See MARK

We can add compile-time options or server variables to control this
process if desired, but they should be localized and only come into play
in the calculation function.


>>>>+            mystrlcpy(namebuf, tile_types[type].terrain_name,
>>>>+ sizeof(tile_types[type].terrain_name)); >>>>+
>>>>+           /* transform to lower case */
>>>>+           for (k = 0; k < strlen(namebuf); k++) {
>>>>+             namebuf[k] = tolower(namebuf[k]);
>>>>+           }
>>>>
>>>>
>>>Doing this transformation for every city in every nation seems
>>>sub-optimal. Although I don't suspect a performance difference the
>>>reader wonders why this is done every time.
>>>
>>Indeed.  On the other hand, caching the value would require code that
>>was much less localized, so it'd make things less clear IMO.
>>
>
>>One solution might be to drop the tolower() stuff altogether and say
>>that the terrain description in the ruleset must match what the game
>>has. This might be more "correct" since the game could theoretically
>>have "Ocean" and "ocean" as two different terrain names (although
>>this would be ludicrous).
>>
>
> Sounds sane.

Hmmm. Instead I've replaced the whole thing with mystrcasecmp to compare the strings case-independently. This isn't really any faster but the code is much cleaner.

>>Also, I think we should make naturalcitynames a server variable, since
>>it'll be easy to disable the functionality.  I haven't done this yet,
>>though.
>>
>>New patch attached.  I haven't tested it much since making the changes.

And here's a new one.

Changes:

1. Changed comments in evaluation of city priority. See above. I also changed a 10 to a 9 in the evaluation algorithm.

2.  Removed all tolower() junk, replaced with mystrcasecmp.  See above.

3. Added naturalcitynames server variable. I added the entry itself to stdinhand, plus the natural_city_names field to the game structure, the #definitions of min/default/max values, and the initialization to the default value. Then, I changed the city evaluation function to use this variable. Note that if naturalcitynames is disabled, the code will still run through and evaluate _each_ city, even though it's the first city that will always be chosen. This is inefficient, but (1) it's quite insignificant and (2) it allows us to localize all future options to within evalutate_city_name_priority(). For instance, we could allow different values of naturalcitynames to have different degrees of terrain association (basically what Raimar described before) without changing any (city naming) code outside of the evaluation function. Also, I've allowed the option to be changed mid-game, which should be fine.

4. The default values are no longer initialized when the ruleset is loaded. Instead they are assembled when it is searched to find the "best" name.

All in all, I think it's ready now.

Once concern is the data segment, which has gotten only minor changes. Shortly I'll provide a patch which reverses the changes from the original natural city naming system without adding any new natural-naming data.

Also, Takacs Gabor said he had made nation rulesets for 12 different nations, but when I tried to respond to him all my e-mails have bounced. Takacs, are you reading this? Assuming the rulesets are geographically accurate, they should be moved into CVS sometime after the code is changed. The initial change should just fix the code and revert the rulesets IMO.

jason
Index: common/game.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/game.c,v
retrieving revision 1.116
diff -u -r1.116 game.c
--- common/game.c       2002/01/09 21:48:08     1.116
+++ common/game.c       2002/01/10 01:52:14
@@ -687,6 +687,7 @@
   game.citymindist = GAME_DEFAULT_CITYMINDIST;
   game.civilwarsize= GAME_DEFAULT_CIVILWARSIZE;
   game.savepalace  = GAME_DEFAULT_SAVEPALACE;
+  game.natural_city_names = GAME_DEFAULT_NATURALCITYNAMES;
   game.unhappysize = GAME_DEFAULT_UNHAPPYSIZE;
   game.angrycitizen= GAME_DEFAULT_ANGRYCITIZEN;
   game.foodbox     = GAME_DEFAULT_FOODBOX;
Index: common/game.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/game.h,v
retrieving revision 1.93
diff -u -r1.93 game.h
--- common/game.h       2002/01/09 21:48:09     1.93
+++ common/game.h       2002/01/10 01:52:14
@@ -124,6 +124,7 @@
   int sewer_size;
   int add_to_size_limit;
   int savepalace;
+  int natural_city_names;
   int spacerace;
   int turnblock;
   int fixedlength;
@@ -341,6 +342,10 @@
 #define GAME_MIN_SAVEPALACE          0
 #define GAME_MAX_SAVEPALACE          1
 #define GAME_DEFAULT_SAVEPALACE      1
+
+#define GAME_MIN_NATURALCITYNAMES     0
+#define GAME_MAX_NATURALCITYNAMES     1
+#define GAME_DEFAULT_NATURALCITYNAMES 1
 
 #define GAME_DEFAULT_FOODBOX         10
 #define GAME_MIN_FOODBOX             5
Index: common/nation.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.h,v
retrieving revision 1.8
diff -u -r1.8 nation.h
--- common/nation.h     2001/12/25 23:58:12     1.8
+++ common/nation.h     2002/01/10 01:52:14
@@ -27,6 +27,26 @@
 enum advisor_type {ADV_ISLAND, ADV_MILITARY, ADV_TRADE, ADV_SCIENCE, 
ADV_FOREIGN, 
                    ADV_ATTITUDE, ADV_DOMESTIC, ADV_LAST};
 
+/*
+ * The city_name structure holds information about a default choice for
+ * the city name.  The "name" field is, of course, just the name for
+ * the city.  The "river" and "terrain" fields are entries recording
+ * whether the terrain is present near the city - we give higher priority
+ * to cities which have matching terrain.  In the case of a river we only
+ * care if the city is _on_ the river, for other terrain features we give
+ * the bonus if the city is close to the terrain.  Both of these entries
+ * may hold a value of 0 (no preference), 1 (city likes the terrain), or -1
+ * (city doesn't like the terrain).
+ *
+ * This is controlled through the nation's ruleset like this:
+ *   cities = "Washington (ocean, river, swamp)", "New York (!mountains)"
+ */
+struct city_name {
+       char* name;
+       int river;
+       int terrain[T_COUNT];   
+};
+
 struct nation_type {
   char name[MAX_LEN_NAME];
   char name_plural[MAX_LEN_NAME];
@@ -36,11 +56,7 @@
   char *leader_name[MAX_NUM_LEADERS];
   int  leader_is_male[MAX_NUM_LEADERS];
   int city_style;
-  char **default_city_names;
-  char **default_rcity_names;          /* river city names */
-  char **default_crcity_names;         /* coastal-river city names */
-  char **default_ccity_names;          /* coastal city names */
-  char **default_tcity_names[T_COUNT]; /* terrain-specific city names */
+  struct city_name *city_names;                /* The default city names. */
   struct Sprite *flag_sprite;
 
   /* untranslated copies: */
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.149
diff -u -r1.149 citytools.c
--- server/citytools.c  2002/01/09 21:48:10     1.149
+++ server/citytools.c  2002/01/10 01:52:20
@@ -52,13 +52,119 @@
 
 #include "citytools.h"
 
+static int evaluate_city_name_priority(int x, int y,
+                                      struct city_name *city_name,
+                                      int default_priority);
+static char *search_for_city_name(int x, int y, struct city_name *city_names);
 static void server_set_tile_city(struct city *pcity, int city_x, int city_y,
                                 enum city_tile_type type);
 
-char **misc_city_names; 
-int num_misc_city_names;
+struct city_name *misc_city_names;
 
 /****************************************************************
+Returns the priority of the city name at the given position,
+using its own internal algorithm.
+
+This function takes into account game.natural_city_names, and
+should be able to deal with any future options we want to add.
+*****************************************************************/
+static int evaluate_city_name_priority(int x, int y,
+                                      struct city_name *city_name,
+                                      int default_priority)
+{
+  /* Lower values are better. */
+  int value = default_priority;
+  int type, goodness;
+
+  /*
+   * If natural city names aren't being used, we just return the
+   * base value.  This will have the effect of the first-listed
+   * city being used.  We do this here rather than special-casing
+   * it elewhere because this localizes everything to this
+   * function, even though it's a bit inefficient.
+   */
+  if (!game.natural_city_names) {
+    return value;
+  }
+
+  /*
+   * Assuming we're using the natural city naming system, we use
+   * an internal alorithm to calculate the priority of each name.
+   * It's a pretty fuzzy algorithm; we basically do the following:
+   *   - Change the priority scale from 0..n to 10..n+10.  This means
+   *     each successive city is 10% lower priority than the first.
+   *   - Multiply by a semi-random number.  This has the effect of
+   *     decreasing rounding errors in the successive calculations,
+   *     as well as introducing a smallish random effect.
+   *   - Check over all the applicable terrains, and
+   *     multiply or divide the priority based on whether or not
+   *     the terrain matches.  See comment below.
+   */
+
+  value += 10;
+  value *= 10 + myrand(5);
+
+  /*
+   * "terrain" is -1 if we don't have the terrain, 1 if  we do.
+   * "goodness" therefore becomes positive if we like the terrain,
+   * negative if we don't.
+   *
+   * The reason we multiply as well as divide the value is so
+   * that cities that don't care what terrain they are on (which
+   * is the default) will be left in the middle of the pack.  If
+   * we _only_ multiplied (or divided), then cities that had more
+   * terrain labels would have their priorities hurt (or helped).
+   */
+  goodness = (map_get_special(x, y) & S_RIVER) ?
+             city_name->river : -city_name->river;
+  if (goodness > 0) {
+    value = (float)value / 1.4;
+  } else if (goodness < 0) {
+    value = (float)value * 1.4;
+  }
+
+  for (type = T_FIRST; type < T_COUNT; type++) {
+    /* Now we do the same for every available terrain. */
+    goodness = is_terrain_near_tile(x, y, type) ?
+                city_name->terrain[type] : -city_name->terrain[type];
+    if (goodness > 0) {
+      value = (float)value / 1.4;
+    } else if (goodness < 0) {
+      value = (float)value * 1.4;
+    }
+  }
+
+  return value;        
+}
+
+/****************************************************************
+Searches through a city name list (a struct city_name array)
+to pick the best available city name, and returns a pointer to
+it.  The function checks if the city name is available and calls
+evaluate_city_name_priority to determine the priority of the
+city name.  If the list has no valid entries in it, NULL will be
+returned.
+*****************************************************************/
+static char *search_for_city_name(int x, int y, struct city_name *city_names)
+{
+  int choice, best_value = -1;
+  char* best_name = NULL;
+
+  for (choice = 0; city_names[choice].name; choice++) {
+    if (!game_find_city_by_name(city_names[choice].name)) {
+      int value = evaluate_city_name_priority(x, y, &city_names[choice], 
choice);
+
+      if (best_value == -1 || value < best_value) {
+        best_value = value;
+        best_name = city_names[choice].name;
+      }
+    }
+  }
+
+  return best_name;
+}
+
+/****************************************************************
 Come up with a default name when a new city is about to be built.
 Handle running out of names etc. gracefully.  Maybe we should keep
 track of which names have been rejected by the player, so that we do
@@ -69,9 +175,10 @@
 *****************************************************************/
 char *city_name_suggestion(struct player *pplayer, int x, int y)
 {
-  char **nptr;
-  int i, j;
+  char *name;
+  int i;
   struct nation_type *nation = get_nation_by_plr(pplayer);
+  /* tempname must be static because it's returned below. */
   static char tempname[MAX_LEN_NAME];
 
   static const int num_tiles = MAP_MAX_WIDTH * MAP_MAX_HEIGHT; 
@@ -81,62 +188,22 @@
   
   CHECK_MAP_POS(x,y);
 
-#define SEARCH_AND_RETURN_CITY_NAME(list)   \
-    for(nptr=list; *nptr; nptr++) {         \
-      if(!game_find_city_by_name(*nptr)) {  \
-        return *nptr;                       \
-      }                                     \
-    }
-
-  /* 
-   * Use a special name if the tile has a river, is coastal or there
-   * is an available name depending on the terrain.
-   */ 
-
-  /* deal with rivers */
-  if (map_get_special(x, y) & S_RIVER) {
-    if (is_terrain_near_tile(x, y, T_OCEAN)) {
-      /* coastal river */
-      SEARCH_AND_RETURN_CITY_NAME(nation->default_crcity_names);
-    } else {
-      /* non-coastal river */
-      SEARCH_AND_RETURN_CITY_NAME(nation->default_rcity_names);
-    }
-  }
-
   /* coastal */
-  if (is_terrain_near_tile(x, y, T_OCEAN)) {
-    SEARCH_AND_RETURN_CITY_NAME(nation->default_ccity_names);
-  }
-  
-  /* check terrain type */
-  SEARCH_AND_RETURN_CITY_NAME(nation->
-                             default_tcity_names[map_get_terrain(x, y)]);
+  name = search_for_city_name(x, y, nation->city_names);
+  if (name)
+    return name;
+
+  name = search_for_city_name(x, y, misc_city_names);
+  if (name)
+    return name;
 
-  /* we haven't found a name: it's a normal tile or they're all used */
-  SEARCH_AND_RETURN_CITY_NAME(nation->default_city_names);
-
-#undef SEARCH_AND_RETURN_CITY_NAME
-
-  if (num_misc_city_names > 0) {
-    j = myrand(num_misc_city_names);
-  
-    for (i = 0; i < num_misc_city_names; i++) {
-      if (j >= num_misc_city_names) {
-       j = 0;
-      }
-      if (!game_find_city_by_name(misc_city_names[j])) 
-       return misc_city_names[j];
-      j++;
-    }
-  }
-
   for (i = 1; i <= num_tiles; i++ ) {
     my_snprintf(tempname, MAX_LEN_NAME, _("City no. %d"), i);
-    if (!game_find_city_by_name(tempname)) 
-      return tempname;
+    if (!game_find_city_by_name(tempname))
+      return (tempname);
   }
-  
+
+  assert(0);
   return _("A poorly-named city");
 }
 
Index: server/citytools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.h,v
retrieving revision 1.33
diff -u -r1.33 citytools.h
--- server/citytools.h  2001/12/06 11:59:07     1.33
+++ server/citytools.h  2002/01/10 01:52:21
@@ -15,6 +15,7 @@
 
 #include "packets.h"
 #include "city.h"
+#include "nation.h" /* for struct city_name */
 
 #define FOOD_WEIGHTING 19
 #define SHIELD_WEIGHTING 17
@@ -81,8 +82,7 @@
                         int target, int is_unit, int event);
 
 char *city_name_suggestion(struct player *pplayer, int x, int y);
-extern char **misc_city_names; 
-extern int num_misc_city_names;
+extern struct city_name *misc_city_names;
 
 
 int city_can_work_tile(struct city *pcity, int city_x, int city_y);
Index: server/ruleset.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/ruleset.c,v
retrieving revision 1.84
diff -u -r1.84 ruleset.c
--- server/ruleset.c    2002/01/09 21:48:11     1.84
+++ server/ruleset.c    2002/01/10 01:52:23
@@ -31,12 +31,12 @@
 #include "nation.h"
 #include "packets.h"
 #include "registry.h"
+#include "shared.h"
 #include "support.h"
 #include "tech.h"
 #include "unit.h"
 
 #include "citytools.h"
-
 #include "ruleset.h"
 
 static const char name_too_long[] = "Name \"%s\" too long; truncating.";
@@ -74,6 +74,9 @@
 static void load_terrain_names(struct section_file *file);
 static void load_citystyle_names(struct section_file *file);
 static void load_nation_names(struct section_file *file);
+static struct city_name* load_city_name_list(struct section_file *file,
+                                            char *secfile_str1,
+                                            char *secfile_str2);
 
 static void load_ruleset_techs(struct section_file *file);
 static void load_ruleset_units(struct section_file *file);
@@ -1798,6 +1801,136 @@
 }
 
 /**************************************************************************
+  This function loads a city name list from a section file.  The file and
+  two section names (which will be concatenated) are passed in.  The
+  malloc'ed city name list (which is all filled out) will be returned.
+**************************************************************************/
+static struct city_name* load_city_name_list(struct section_file *file,
+                                            char *secfile_str1,
+                                            char *secfile_str2)
+{
+  char **cities;
+  int dim, j;
+  struct city_name *city_names;
+  int value;
+
+  /* First we read the strings from the section file. */
+  cities = secfile_lookup_str_vec(file, &dim, "%s%s",
+                                 secfile_str1, secfile_str2);
+
+  /*
+   * Now we allocate enough room in the city_names array to store
+   * all the name data.  The array is NULL-terminated by
+   * having a NULL name at the end.
+   */
+  city_names = fc_calloc(dim + 1, sizeof(struct city_name));
+  city_names[dim].name = NULL;
+
+  /*
+   * Each string will be of the form
+   * "<cityname> (<label>, <label>, ...)".  The cityname is just the
+   * name for this city, while each "label" matches a terrain type
+   * for the city (or "river"), with a preceeding !, -, or ~ to negate
+   * it.  The parentheses are optional (but necessary to have the
+   * settings, of course).  Our job is now to parse this into the
+   * city_name structure.
+   */
+  for (j = 0, value=1; j < dim; j++, value++) {
+    /* This whole thing should be handled more succinctly. */
+    char *name = cities[j], *next;
+
+    /*
+     * Now we wish to determine values for all of the city labels.
+     * A value of 0 means no preference (which is necessary so that
+     * the use of this is optional); -1 means the label is negated
+     * and 1 means it's labelled.  Mostly the parsing just involves
+     * a lot of ugly string handling...
+     */
+    memset(&city_names[j].terrain[0], 0,
+          T_COUNT * sizeof(city_names[j].terrain[0]));
+    city_names[j].river = 0;
+
+    name = strchr(name, '(');
+    if (name) {
+      /*
+       * 0-terminate the original string, then find the
+       * close-parenthesis so that we can make sure we stop there.
+       */
+      *name = 0;
+      name++;
+      next = strchr(name, ')');
+      assert(next);
+      *next = 0;
+
+      /* Handle the labels one at a time. */
+      do {
+       int setting;
+
+       next = strchr(name, ',');
+       if (next) {
+         *next = 0;
+       }
+       remove_leading_trailing_spaces(name);
+       
+       /*
+        * The ! is used to mark a negative, which is recorded
+        * with a -1.  Otherwise we use a 1.  '-' and '~' have
+        * the same meaning.
+        */
+       if (*name == '!' || *name == '-' || *name == '~') {
+         name++;
+         setting = -1;
+       } else {
+         setting = 1;
+       }
+       
+       if (!strcmp(name, "river")) {
+         city_names[j].river = setting;
+       } else {
+         /* "handled" tracks whether we find a match (for error handling) */
+         int handled = 0;
+         enum tile_terrain_type type;
+       
+         for (type = T_FIRST; type < T_COUNT && !handled; type++) {
+            /*
+             * Note that at this time (before a call to
+             * translate_data_names) the terrain_name fields contains an
+             * untranslated string.  Note that name of T_RIVER is "".
+             * However this is not a problem because we take care of rivers
+             * separately.
+             */
+           if (!mystrcasecmp(name, tile_types[type].terrain_name)) {
+             city_names[j].terrain[type] = setting;
+             handled = 1;
+           }
+         }
+         if (!handled) {
+           freelog(LOG_ERROR, "Unreadable terrain description %s "
+                   "in city name ruleset \"%s%s\" - skipping it.",
+                   name, secfile_str1, secfile_str2);
+           assert(0);
+         }
+       }
+       name = next ? next+1 : NULL;
+      } while (name && *name);
+    }
+    remove_leading_trailing_spaces(cities[j]);
+    city_names[j].name = mystrdup(cities[j]);
+    if (check_name(city_names[j].name)) {
+      freelog(LOG_ERROR, "City name %s in ruleset for %s%s is too long "
+             "- shortening it.",
+              city_names[j].name, secfile_str1, secfile_str2);
+      assert(0);
+      city_names[j].name[MAX_LEN_NAME - 1] = 0;
+    }
+  }
+  if (cities) {
+    free(cities);
+  }
+  return city_names;
+}
+
+/**************************************************************************
 Load nations.ruleset file
 **************************************************************************/
 static void load_ruleset_nations(struct section_file *file)
@@ -1807,9 +1940,8 @@
   struct government *gov;
   int *res, dim, val, i, j, nval;
   char temp_name[MAX_LEN_NAME];
-  char **cities, **techs, **leaders, **sec;
+  char **techs, **leaders, **sec;
   const char *filename = secfile_filename(file);
-  enum tile_terrain_type type;
 
   datafile_options = check_ruleset_capabilities(file, "+1.9", filename);
 
@@ -2017,72 +2149,13 @@
     }
     pl->goals.government = val;
 
-#define BASE_READ_CITY_NAME_LIST(target,format,arg1,arg2,arg3)  \
-  cities = secfile_lookup_str_vec(file, &dim, format,           \
-                                  arg1, arg2, arg3);            \
-  target = fc_calloc(dim + 1, sizeof(char *));                  \
-  target[dim] = NULL;                                           \
-  for (j = 0; j < dim; j++) {                                   \
-    target[j] = mystrdup(cities[j]);                            \
-    if (check_name(cities[j])) {                                \
-      target[j][MAX_LEN_NAME - 1] = 0;                          \
-    }                                                           \
-  }                                                             \
-  if (cities) {                                                 \
-    free(cities);                                               \
-  }
-
-#define READ_CITY_NAME_LIST(target_field,format,arg)            \
-  BASE_READ_CITY_NAME_LIST(pl->target_field, "%s." format "%s", \
-                           sec[i], arg, "cities")
-
     /* read "normal" city names */
-    READ_CITY_NAME_LIST(default_city_names, "%s", "");
-
-    /* read river city names */
-    READ_CITY_NAME_LIST(default_rcity_names, "%s", "river_");
-
-    /* read coastal-river city names */
-    READ_CITY_NAME_LIST(default_crcity_names, "%s","coastal_river_");
-
-    /* read coastal city names */
-    READ_CITY_NAME_LIST(default_ccity_names, "%s", "coastal_");
-
-    /* 
-     * Read terrain-specific city names. 
-     */    
-    for (type = T_FIRST; type < T_COUNT; type++) {
-      int k;
-      char namebuf[MAX_LEN_NAME];
 
-      /*
-       * Note that at this time (before a call to
-       * translate_data_names) the terrain_name fields contains an
-       * untranslated string. Note that name of T_RIVER is "". However
-       * this is not a problem because we take care of this using
-       * default_rcity_names.
-       */
-      mystrlcpy(namebuf, tile_types[type].terrain_name,
-               sizeof(tile_types[type].terrain_name));
-
-      /* transform to lower case */
-      for (k = 0; k < strlen(namebuf); k++) {
-       namebuf[k] = tolower(namebuf[k]);
-      }
-
-      READ_CITY_NAME_LIST(default_tcity_names[type], "%s_", namebuf);
-    }
+    pl->city_names = load_city_name_list(file, sec[i], ".cities");
   }
 
   /* read miscellaneous city names */
-  BASE_READ_CITY_NAME_LIST(misc_city_names, "misc.cities%s%s%s", "", "",
-                          "");
-
-  /* dim is set in BASE_READ_CITY_NAME_LIST */
-  num_misc_city_names = dim;
-
-#undef READ_CITY_NAME_LIST
-#undef BASE_READ_CITY_NAME_LIST
+  misc_city_names = load_city_name_list(file, "misc.cities", "");
 
   free(sec);
   section_file_check_unused(file, filename);
Index: server/stdinhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/stdinhand.c,v
retrieving revision 1.198
diff -u -r1.198 stdinhand.c
--- server/stdinhand.c  2002/01/07 21:13:07     1.198
+++ server/stdinhand.c  2002/01/10 01:52:26
@@ -706,6 +706,14 @@
        "automatically rebuilt for free in another randomly choosed "
        "city, regardless on the knowledge of Masonry.") },
 
+  { "naturalcitynames", &game.natural_city_names, NULL, NULL,
+    SSET_RULES_FLEXIBLE, SSET_TO_CLIENT,
+    GAME_MIN_NATURALCITYNAMES, GAME_MAX_NATURALCITYNAMES,
+       GAME_DEFAULT_NATURALCITYNAMES,
+    N_("Whether to use natural city names"),
+    N_("If enabled, the default city names will be determined based on "
+       "the surrounding terrain.") },
+
 /* Meta options: these don't affect the internal rules of the game, but
  * do affect players.  Also options which only produce extra server
  * "output" and don't affect the actual game.

[Prev in Thread] Current Thread [Next in Thread]