[Freeciv-Dev] Re: [Patch] Make get_city_*_bonus public

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> The changes which are required for the dynamic CMA patch as a seperate
> one. It makes the three methods public, adds some comments and cleans
> them up.

It's a good clean patch. I've got a few questions:


>  /**************************************************************************
> -...
> + Return the factor (in %) by which the shield should be multiplied.
>  **************************************************************************/

I would prefer:

Return the %(> 100) by which the city's base shields should be multiplied. 

+ Return the  % ( > 100) by which the city's base tax and luxury should be
+ multiplied.

The same for this line.

+  Return the  % ( > 100) by which the city's base science should be
multiplied.

> +int get_city_shield_bonus(struct city *pcity)
>  {
> -  int tmp = 0;
> +  int shield_bonus = 100;
> +
>    if (city_got_building(pcity, B_FACTORY)) {
> -    if (city_got_building(pcity, B_MFG))
> -      tmp = 100;
> -    else
> -      tmp = 50;
> +    shield_bonus += 50;
> +    if (city_got_building(pcity, B_MFG)) {
> +      shield_bonus += 50;
> +    }

What happens if a city sells the factory, and still has a mfg plant? Substitute
in bank/stock exchange/university/research center for factory. It seems the
freeciv manual is wrong about the behaviour. I always thought that even after
selling a marketplace your bank would be bringing in money.
  
>      if (city_affected_by_wonder(pcity, B_HOOVER) ||
>       city_got_building(pcity, B_POWER) ||
>       city_got_building(pcity, B_HYDRO) ||
>       city_got_building(pcity, B_NUCLEAR)) {
> -      tmp = (3 * tmp) / 2;
> +      shield_bonus = 100 + (3 * (shield_bonus - 100)) / 2;
>      }
>    }
>  
> -  pcity->shield_bonus = tmp + 100; <-

What's happening above? This line seems fishy to me. I realise you got rid of
it, but what was it doing originally?

Aloha,
RK.

Real education happens when you pick up a fact here, and another fact there,
and put them together and get an insight.
{Isis Publishing interview, Apr.-Jun., 2000, Terry Pratchett}

__________________________________________________
Do You Yahoo!?
Yahoo! Health - your guide to health and wellness
http://health.yahoo.com