[Freeciv-Dev] Re: [Patch] Fortress with enhanced vision (watchtower-like)

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

Hi Bert,

 --- Bert Buchholz <bertbuchholz@xxxxxx> wrote: 
> Hi,
> 
> Patch against cvs from Fri, Aug-24, somewhen in the evening :-) 
> This is now the watchtower "included" in a fortress, because most

Here is my promised report on your patch.  I detected several problems
with it:

1. Falk is right: you should check for is_ground_unit more
systematically.
AFAIR, there was something about ships going into rivers so the fact that
there is fortress on a square does not guarantee that there are no ships
there.

2. Consider the situation: players A and B are allied and player A builds
a fortress on a square occupied by Bs unit.  Your patch would not handle
this situation properly.

3. Thue was very right when he said that fog system is very touchy.  If
you calculate it incorrectly once, it will stay this way.  So please
check for all cases of potential vision changes!!  You can do using
freeciv's source browsing system.  For example 
  http://www.freeciv.org/lxr/ident?v=cvs&i=fog_area
reveals that you forgot to upgrade unittools.c around line 1541 (unit
upgrading).

> So, if anything's missing, or other failures (which are of course
> intentional ;-), please tell me and I'll try to kick them out, or you

You must be absolutely sure that your patch updates all the relevant bits
of code.  Otherwise we will have many fog bugs in seemingly random
places.

All in all, I would rate your patch as "good development with quite a lot
of problems" (it's 2 in Raimar's system init?).

Comments on other topics:
1. Things like watchtower_vision and watchtower_extra_vision should
definitely go into the rulesets, but not necessarily in this patch.
2. The shape of vision area is a topic completely independent of
watchtower business and should be discussed as such (i.e. please change
subject of the email when writing about vision areas).

G.

____________________________________________________________
Do You Yahoo!?
Get your free @yahoo.co.uk address at http://mail.yahoo.co.uk
or your free @yahoo.ie address at http://mail.yahoo.ie