[Freeciv-Dev] Re: [PATCH] advdomestic.c cleanup (PR#1149)

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- Petr Baudis <pasky@xxxxxxxxxxx> wrote:

> It's clear. When bx and by is zero, best is zero as well. Always. It's
> evident
> that this code was much more complicated originally, but got reduced and
> simplified.  And this test has no more mean. Even bx and by is not needed
> anymore at all :).

Good. I agree with the change.

Impr_happy_val maybe should have a comment above it saying
it is improvement_happy_value.
 
> Attached revised patch. I changed building_value to impr_happy_val - it maybe
> looks more unfriendly at the first sight, but I believe it's more descriptive
> and less confusing.
> 


> +/**************************************************************************
> +Get value of best usable title on city map.
> +**************************************************************************/
>  static int ai_best_tile_value(struct city *pcity)
>  {

Looks good.


> +/**************************************************************************
> +Returns the value of the improvement for keeping of order in the city.
> +
> +Improvements which increase happiness have two benefits. They can prevent
> +cities from falling into disorder, and free the population changed into
> elvis
> +specialists (so you can work tiles with value like tileval with them). 
> Happy
> +is number of people the improvement would make happy.
> +
> +This function lets you decide if the happiness improvement such as the
> Temple,
> +Cathedral or WoW is worth the cost of the elvises keeping your citizens
> content
> +or the cost of the potential disorder (basically how much citizens would be
> +unhappy * SADVAL).
> +
> +Note that an elvis citizen is a parasite. The fewer you have, the more
> +food/prod/trade your city produces.
> +**************************************************************************/

Very nice comments. I see Ross has had an effect.

> +static int impr_happy_val(int happy, struct city *pcity, int tileval)

Looks ok.

> +/**************************************************************************
> +Return a weighted value for a city's Ocean tiles
> +(i.e. based on the total number and number actively worked).
> +**************************************************************************/

Formatting changes only, so it is fine.

> +/**************************************************************************
> +Number of road tiles actively worked.
> +**************************************************************************/

> +/**************************************************************************
> +Number of farmland tiles actively worked.
> +**************************************************************************/

All of the above are good.


> +static int pollution_benefit(struct player *pplayer, struct city *pcity,
> +                          Impr_Type_id impr_type)
> 

Pollution benefit I have not reviewed. Can someone else please send in 
another review?


__________________________________________________
Do You Yahoo!?
Check out Yahoo! Shopping and Yahoo! Auctions for all of
your unique holiday gifts! Buy at http://shopping.yahoo.com
or bid at http://auctions.yahoo.com