[Freeciv-Dev] Re: Pop cost patch (resending via bug system) (PR#897)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Aug 22, 2001 at 03:02:24PM -0700, Arien Malec wrote:
> OK -- attached is a patch that cleans up handle_unit_build_city considerably,
> but may move ugliness to other places:
> 
> 1) can_unit_add_to_city and can_unit_build_city are gone, replaced by
> can_unit_add_or_build_city
> 2) can_unit_add_or_build_city centralizes all the checking and logic for city
> adding/building, and centralizes all the consistency checking
> 3) handle_unit_build_city is broken into three cases: adding, building, and
> error, each handled by static functions

Looks good.

> Issues:
> The static functions trust that can_unit_or_build_city does indeed do all the
> consistency checking. For instance, if can_unit_add_or_build_city returns
> AB_NO_AQUEDUCT, city_add_build_error doesn't bother to check that pcity !=
> NULL. It may be appropriate to sprinkle some assertions about???

city_add_or_build_error is a static method. So there should be no
problems. Add a comment in header and it would be fine. If there is
some error we get a null exception.

> diff -u -r1.6 cities.ruleset
> --- data/default/cities.ruleset       2001/05/24 22:12:09     1.6
> +++ data/default/cities.ruleset       2001/08/22 21:51:25
> @@ -12,7 +12,7 @@
>  options="1.9"
>
>  [parameters]
> -add_to_size_limit  = 8               ; cities >= this cannot be added to.
> +add_to_size_limit  = 18              ; cities >= this cannot be added to.
>
>  ;
>  ; City styles define the way cities are drawn

Whats this?

> +static void city_build (struct player *pplayer, struct unit *punit, char 
> *name)
> +{
> +  char *city_name;
> +  if(!(city_name=get_sane_name(name))) {

I don't like assigment in a if condition. It really looks like we need
a language style person.

> the_names_of_things_are_long

Thats ok.

Some comments in the method headers and we are finished.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 1 + 1 = 3, for large values of 1