[Freeciv-Dev] Re: (PR#8754) effects patch

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8754 >

On Thu, Jul 08, 2004 at 11:38:09AM -0700, Vasco Alexandre da Silva Costa wrote:
> 
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8754 >
> 
> === Complex things that should IMHO be done post commit:
> 
> * generalized sources cache (allows non-wonders with larger
>   than city range, makes effect range checks faster).

I must disagree here. This needs to be done before the majority of the code
is committed mainly because I foresee that a bunch of committed code will
then have to be changed all over again. Then again I could be convinced
otherwise, perhaps Vasco has a detailed plan on how to do this...
 
> client/packhand.c: uses B_PALACE and B_CITY in handle_city_short_info.
 
yeah, this is nasty. There is also a ticket somwhere that deals with the
client's problems with not getting rid of stale palaces.

> If there are any keywords you would like to rename, better rename
> them sooner rather than later.

I have only skimmed the patch, so take these with a grain of salt. I'll
look harder after the middle of next week.

leonardo and marco_polo functions should be renamed.
city_gan_grow_to() has two function prototypes.
I am so not a fan of game.palace_improvement (though I'm not sure yet what
   you're attempting to accomplish here).
The above in is_captial() worries me.
I see there's been no movement on {Sea|Land|Etc}_Defend...
The use of the variable name "elt" is rather vague and means nothing to me.
Why Island->Continent?

-mike