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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Aug 21, 2001 at 12:52:16PM -0700, Arien Malec wrote:
> OK, suggested changes applied, *and* I managed to get unittype.c to have two
> and only two extra newlines at the end, so there are no extraneous diffs :-)

>    pcity = map_get_city(punit->x, punit->y);
> -
> +  new_pop = pcity->size + unit_pop_value(punit->type);
> +
>    if(!pcity)
>      return 0;

This looks bad. pcity is checked afterwards.

>    if(improvement_exists(B_AQUEDUCT)
>       && !city_got_building(pcity, B_AQUEDUCT)
> -     && pcity->size >= game.aqueduct_size)
> +     && new_pop  >= game.aqueduct_size)
>      return 0;

Consider game.aqueduct_size==8. The old version will return 0 if the
city is size 8 or above. Your version will forbid it starting at size
7.

Same here:
>    if(improvement_exists(B_SEWER)
>       && !city_got_building(pcity, B_SEWER)
> -     && pcity->size >= game.sewer_size)
> +     && new_pop >= game.sewer_size)
>      return 0;

> +     /* Should we disband the city? -- Massimo */
> +     if (pcity->size==unit_pop_value (pcity->currently_building) &&
> +         (pcity->city_options & ((1<<CITYO_DISBAND)))) {

Superfluous ()'s. Maybe a
"assert(pcity->size>=unit_pop_value(pcity->currently_building));"? Or
some asserts in city_reduce_size().

> -      if(pcity->size >= game.add_to_size_limit) {
> +      if(pcity->size + unit_pop_value(punit->type) >= 
> game.add_to_size_limit) {

> -      && pcity->size >= game.aqueduct_size) {
> +      && pcity->size + unit_pop_value(punit->type) >= game.aqueduct_size) {

> -           && pcity->size >= game.sewer_size) {
> +           && pcity->size + unit_pop_value(punit->type) > game.sewer_size) {

Same off by one errors as above.

As a side note: (as a seperate patch) what do you think about
something like this:

enum add_to_city_error
{
  OK, NO_UNIT_WITH_POP, CITY_TO_BIG, NO_AQUEDUCT,...;
};

enum add_to_city_error can_unit_add_to_city(struct unit *punit);

And than remove this duplicated code in handle_unit_build_city.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "SIGDANGER - The System is likely to crash soon"