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

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
<snip>
> /**************************************************************************
> > > -...
> > > + 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. 
> 
> I can't and wouldn't ensure that the factor is always >=100. It may be
> quite possible that there is a building which increase the shield
> output by 50% and reduce the science output by 50%.

Thinking of the future here. Currently you can indeed give such an ironclad
guarantee. I take it you're thinking about the AC people's patches?

> > > +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?
> 
> It doesn't get any bonus.

I think this is not a good behaviour. As a player, I would expect that selling
my marketplace/library/factory would not result in loss of all my other
improvements along those lines. A buglet. At the very least, if the improvement
does nothing it should not cost me upkeep.

> > 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.
> 
> This isn't a change my patch made.

I know. I don't like this behaviour. Think about it, all a spy has to do is
sabotage 1 building and two others are now useless, and a productive science
city is now worthless. Pretty stiff penalty. Also a great way to throw a city
into disorder. Take out the temple and it's done for.

> > >      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?
> 
> tmp in the old version was "shield_bonus - 100". The assignment of tmp
> can easily be replaced with shield_bonus. However in the above
> multiplication some not so nice code is necessary.
> 
>       Raimar
> 
> -- 
>  email: rf13@xxxxxxxxxxxxxxxxx
>   What's nice about GUI is that you see what you manipulate.
>   What's bad about GUI is that you can only manipulate what you see.

Aloha,
RK.

In spite of the cost of living, it's still popular.
Laurence J. Peter

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