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

[Jason Short <vze2zq63@xxxxxxxxxxx>]

Erik Sigra wrote:

> > + * This is controlled through the nation's ruleset like this:
> > + *   cities = "Washington (ocrean, river, swamp)", "New York (!mountains)"

> "What is "!mountains" supposed to mean? Is it a C-style negation? If
> so, it is only intuitive to C programmers and not to anyone else. 
> Normal people consider the exclamation sign to increase the meaning
> of something, not negate it. Use another notation like for example 
> "not mountains". I would use the proper negation sign, ¬, which is 
> in at least latin1.

You obviously haven't played enough NetHack. :-)

I think ! is sufficiently standard that I don't really want to code 
parsing of "not " in its place.
But, here's an updated patch.  Changes include:

- Fixed "ocrean" typo.
- Added '-' and '~' as alternatives for '!' negation.
- Changed goodness-calculating expression (GB's recommendation).


jason

? diff
? jason-game.gz
? jason.gz
? old
? rc
? topology
? data/diff
Index: common/nation.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/nation.h,v
retrieving revision 1.7
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).
+ *
+ * This is controlled through the nation's ruleset like this:
+ *   cities = "Washington (ocean, river, swamp)", "New York (!mountains)"
+ */
+struct city_name {
+       char* name;
+       int value;
+       int river;
+       int terrain[T_COUNT];   
+};
+
 struct nation_type {
   char name[MAX_LEN_NAME];
   char name_plural[MAX_LEN_NAME];
@@ -36,11 +58,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.148
diff -u -r1.148 citytools.c
--- server/citytools.c  2001/12/11 16:48:48     1.148
+++ server/citytools.c  2001/12/22 20:22:17
@@ -52,12 +52,78 @@
 
 #include "citytools.h"
 
+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;
 
+
+/****************************************************************
+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);
+
+      /*
+       * Use a special name if the tile has a river or there
+       * is an available name depending on the terrain.
+       */
+
+      /*
+       * "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 integer values are just
+       * approximations, so...
+       *
+       * 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) ?
+                choice->river : -choice->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++) {
+        goodness = is_terrain_near_tile(x, y, type) ?
+                  choice->terrain[type] : -choice->terrain[type];
+        if (goodness > 0) {
+          value = (float)value / 1.4;
+        } else if (goodness < 0) {
+         value = (float)value * 1.4;
+        }
+      }
+
+      if (best_value == -1 || value < best_value) {
+        best_value = value;
+        best_name = 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
@@ -69,9 +135,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 +148,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)]);
-
-  /* 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
+  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;
 
-  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  2001/12/22 20:22:17
@@ -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.83
diff -u -r1.83 ruleset.c
--- server/ruleset.c    2001/12/06 11:59:07     1.83
+++ server/ruleset.c    2001/12/22 20:22:19
@@ -74,6 +74,7 @@
 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);
@@ -1791,6 +1792,184 @@
 }
 
 /**************************************************************************
+  This strips leading and trailing whitespace from a string.
+
+  Surely this capability is provided by some other source???
+**************************************************************************/
+static void strip_space(char* str)
+{
+  char *s1 = str, *s2 = str;
+
+  /* Skip past leading spaces. */
+  while(isspace(*s1)) {
+    s1++;
+  }
+
+  /* Copy over the real text. */
+  while (*s1) {
+    *s2 = *s1;
+    s1++;
+    s2++;
+  }
+
+  /* Backtrack over trailing spaces. */
+  while (s2 > str && isspace(*(s2-1))) {
+    s2--;
+  }
+
+  /* Null-terminate */
+  *s2 = 0;
+}
+
+/**************************************************************************
+  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 ! 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=10; j < dim; j++, value++) {
+    /* TODO: handle this more succinctly. */
+    char *name = cities[j], *next;
+
+    /*
+     * We assign a "base value" to each city based upon its
+     * position in the list.  We arbitrarily start at 10 and
+     * count up - higher values mean lower priority, meaning
+     * the name is less likely to be picked later.
+     */
+    city_names[j].value = value;
+
+    /*
+     * 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 = 1, i;
+       next = strchr(name, ',');
+       if (next)
+         *next = 0;
+       strip_space(name);
+       for (i=0; name[i]; i++)
+         name[i] = tolower(name[i]);
+       
+       /*
+        * 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;
+       }
+       
+       /*
+        * We used to have "coastal" as a special case along
+        * with river, but this is unnecessary since we
+        * also have the "ocean" terrain type.
+        */
+       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++) {
+           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 rivers
+             * separately.
+             */
+            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]);
+           }
+       
+           if (!strcmp(name, namebuf)) {
+             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);
+         }
+       }
+       name = next ? next+1 : NULL;
+      } while (name && *name);
+    }
+    strip_space(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);
+      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)
@@ -1800,9 +1979,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);
 
@@ -2010,72 +2188,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);