Complete.Org: Mailing Lists: Archives: freeciv-dev: April 2002:
[Freeciv-Dev] Re: new_natural_city_names updated again (PR#1127)
Home

[Freeciv-Dev] Re: new_natural_city_names updated again (PR#1127)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: per@xxxxxxxxxxx
Cc: freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: new_natural_city_names updated again (PR#1127)
From: Mike Kaufman <kaufman@xxxxxxxxxxxxxxxxxxxxxx>
Date: Tue, 9 Apr 2002 09:16:41 -0500

On Tue, Apr 09, 2002 at 03:13:55AM -0700, per@xxxxxxxxxxx wrote:
> On Tue, 9 Apr 2002, Jason Short wrote:
> > This is another update of the new natural city names patch to current
> > CVS.  Nothing has really changed; see all the old discussion for an
> > explanation.
> 
> +struct city_name {
> +     char* name;
> +     int river;
> +     int terrain[T_COUNT];
> +};
> 
> Can you make it char name[MAX_LEN_NAME] instead? Fewer allocations
> better.
> 
> -  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. */
> 
> Thank you. I hated those.
> 
> +  if (name)
> +    return name;
> 
> if (name != NULL) {
>   return name;
> }
> 
> +  if (name)
> +    return name;
> 
> Ditto.
> 
> +      } while (name && *name);
> 
> I hate to say it, because I like this a lot myself, but this should be
> 
>   while (name != NULL && strlen(name) > 0)

na, we decided that if (!ptr) and if (ptr) is ok, to avoid lots o' cruft.  
It should be ansi and all that jazz.

-mike

> 
> +  GEN_BOOL("naturalcitynames", game.natural_city_names,
> +           SSET_RULES_FLEXIBLE, SSET_TO_CLIENT,
> +           N_("Whether to use natural city names"),
> +           N_("If enabled, the default city names will be determined based "
> +              "on the surrounding terrain."),
> +           NULL, GAME_DEFAULT_NATURALCITYNAMES)
> +
> 
> IMHO, you can just make this the default. Why wouldn't anyone want this?
> (And even if they didn't want it, would they bother enough to turn it
> off? I doubt it.) I don't like unnecessary "options creep".
> 
> Overall, I think the patch looks good and is a definite improvement. Not
> tested.
> 
> Yours,
> Per
> 
> "While we're all in favor of Arafat doing more to suppress violence in
> the Middle East, we fear that he doesn't have all that much influence on
> the Israeli government. " --Sam Smith
> 
> 


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