Complete.Org: Mailing Lists: Archives: freeciv-dev: December 2001:
[Freeciv-Dev] Re: Is the city names patch good? (PR#1126)
Home

[Freeciv-Dev] Re: Is the city names patch good? (PR#1126)

[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: Is the city names patch good? (PR#1126)
From: jdorje@xxxxxxxxxxxxxxxxxxxxx
Date: Tue, 11 Dec 2001 13:53:35 -0800 (PST)

Raimar Falke wrote:

On Tue, Dec 11, 2001 at 02:38:57PM -0500, Jason Short wrote:

Jason Short wrote:

Upon further thought, I have come to the conclusion that the natural city names sytem _is_ fatally flawed, and should be replaced (preferably by a better natural city name system, perhaps one like I describe above).

The flaw is this: it discards the city order from the ruleset. This will mean that it is impossible to convert the ruleset back to on ordered system. (By "impossible", I mean it would be just as much work as building the ruleset from scratch. It would still be possible to revert to a previous CVS version, but all additions to the ruleset would be lost.)

If the 64-nation limit is relaxed before the next release, then after the release we're going to see an influx of many more nations. And all (or most) of these nations will use natural city naming. It will then become infeasible to change systems at a later date.

All I'm really arguing for is a change in the ruleset syntax (perhaps as I describe above). If somebody (preferably the original author, maybe me) provides a patch to do this, would it be accepted?


I see the problem. However the other problem (putting more info on a
name) is the more important reason (at least for me now). So I will
accept such patches.


Here is a preleminary patch to accomplish the task.


It is intended to accept rulesets of the form

cities =
  "Washington(coastal,river),New York(coastal,river)", ...

However, the string parsing code needs work (it can't handle whitespace or bad capitalization, for one thing).

It should work as-is with the old (pre-natural-names) nation rulesets (do cvs up -D "2 weeks ago" to get this). However, to get real results you need to redo the natural names in the rulesets to match what it wants (I'll provide some separately).

Also, note that you can still label any "terrain" type in the rulesets. This terrain will be associated with the city if the city is "near" it. With a complicated enough ruleset, you could end up getting some pretty well-named cities, I think.

All in all, I think it will do well.  It certainly needs more work, though.

jason
? diff
? freeciv-patches
? other_patches
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/11 21:45:39
@@ -27,6 +27,18 @@
 enum advisor_type {ADV_ISLAND, ADV_MILITARY, ADV_TRADE, ADV_SCIENCE, 
ADV_FOREIGN, 
                    ADV_ATTITUDE, ADV_DOMESTIC, ADV_LAST};
 
+/* The city_name struct holds information about a city name.  The name is
+   fairly obvious; the terrain type is the preferred terrain type for this
+   city; river and coastal should be 1 if the city is generally on a
+   river/coastline.  The "value" is used internally so that earlier-listed
+   cities get preference. */
+struct city_name {
+  char* name;
+  int value;
+  int terrain[T_COUNT];
+  int river, coastal;
+};
+
 struct nation_type {
   char name[MAX_LEN_NAME];
   char name_plural[MAX_LEN_NAME];
@@ -36,11 +48,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;
   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/11 21:45:41
@@ -52,12 +52,62 @@
 
 #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; 
+struct city_name *misc_city_names;
 int num_misc_city_names;
 
+
+
+static char *search_for_city_name(int x, int y, struct city_name *city_names)
+{
+  struct city_name *choice;
+  int best_value = -1;
+  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;
+
+      /*
+       * Use a special name if the tile has a river, is coastal or there
+       * is an available name depending on the terrain.
+       */
+
+      /* It's tempting to give a bonus for the contrapositive too; for
+         instance anything not marked coastal will get a bonus if it's
+         inland.  But this will penalize rulesets that aren't
+         fully-formed because these bonuses may be inaccurate.  Later we
+         may want to allow a !coastal setting to handle this. */
+
+      /* Coastal is _seemingly_ handled by the terrain check below. */
+      if (choice->coastal && is_terrain_near_tile(x, y, T_OCEAN)) {
+        value /= 2;
+      }
+
+      if (choice->river && (map_get_special(x, y) & S_RIVER)) {
+        value /= 2;
+      }
+
+      for (type = T_FIRST; type < T_COUNT; type++) {
+        if (choice->terrain[type] && is_terrain_near_tile(x, y, type)) {
+         value /= 2;
+        }
+      }
+
+      freelog(LOG_ERROR, "Name %s has value %d.", choice->name, value);
+
+      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 +119,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 +132,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/11 21:45:41
@@ -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,7 +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 struct city_name *misc_city_names;
 extern int num_misc_city_names;
 
 
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/11 21:45:43
@@ -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);
@@ -1790,6 +1791,82 @@
   free(sec);
 }
 
+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;
+
+  cities = secfile_lookup_str_vec(file, &dim, "%s%s", secfile_str1, 
secfile_str2);
+
+  city_names = fc_calloc(dim + 1, sizeof(struct city_name));
+  city_names[dim].name = NULL;
+
+  for (j = 0, value=10; j < dim; j++, value++) {
+    /* TODO: handle this more succinctly, and
+       remove whitespace. */
+    char *name = cities[j], *next;
+
+    city_names[j].value = value;
+    memset(&city_names[j].terrain[0], 0, T_COUNT * 
sizeof(city_names[j].terrain[0]));
+    city_names[j].river = city_names[j].coastal = 0;
+
+    name = strchr(name, '(');
+    if (name) {
+      *name = 0;
+      name++;
+      next = strchr(name, ')');
+      assert(next);
+      *next = 0;
+      do {
+       next = strchr(name, ',');
+       if (next)
+         *next = 0;
+       freelog(LOG_ERROR, "Handling marker '%s'.", name);
+       if (!strcmp(name, "river")) {
+         city_names[j].river = 1;
+       } else if (!strcmp(name, "coastal")) {
+         city_names[j].coastal = 1;
+       } else {
+         enum tile_terrain_type type;
+         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 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] = 1;
+           }
+         }
+       }
+       name = next ? next+1 : NULL;
+      } while (name && *name);
+    }
+    city_names[j].name = mystrdup(cities[j]);
+    if (check_name(city_names[j].name)) {
+      city_names[j].name[MAX_LEN_NAME - 1] = 0;
+    }
+  }
+  if (cities) {
+    free(cities);
+  }
+  return city_names;
+}
+
 /**************************************************************************
 Load nations.ruleset file
 **************************************************************************/
@@ -1800,9 +1877,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,66 +2086,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", "", "",
-                          "");
+  misc_city_names = load_city_name_list(file, "misc.cities", "");
 
   /* dim is set in BASE_READ_CITY_NAME_LIST */
   num_misc_city_names = dim;

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