<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40607 >

2008/12/21 Yoav Luft:
> Hi, I think it's pretty much finished, at least the basic functionality.
> The attached .diff file adds the functionality of cities losing
> population when they grow too large due to diseases.

 I found additional comments in RT ticket that were not sent to
mailing list. I looked latest version of the patch,

 So far I only read it through and tested compile.

 Basic functionality is rather simple. Good. Extra penalty from cities
we trade with and had plague seems like unnecessary complication. It
adds realism, but no new value to the game. How often you end with
plague, and how often that happens to trigger another plague anyway. I
would leave that part out, at least for now until basic functionality
has been in use for some time.

 AI effect evaluation seems wrong to me
v += c * 5 + (amount / 5) * pcity->illness + pcity->had_plague * 20;
 - Should return 0 if plague is turned off in ruleset
 - Should return 0 for size 1 city which never can be subject to plague
 - Had_plague does not increase possibility of the future plague
(actually it makes it smaller, as city gets smaller)

 Coding style dictates that variables cannot be declared in the middle
of the block. By removing braces somewhere in cityturn.c you leave
"int id" in the middle of the block.

 Since you are adding new event, not this comment in event.h:
   * Note: If you add a new event, make sure you make a similar change
   * to the events array in common/events.c using GEN_EV,
   * data/stdsoundes and to server/scripting/api.pkg

 - ML

Freeciv-dev mailing list

Reply via email to