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

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Bert Buchholz <bertbuchholz@xxxxxx> wrote: 

> with unit_profits_of_watchtower() as Falk suggested. Regarding this
> ships-on-river issue: This isn't implemented yet, right? Or did I miss

not officially.
but I think Ross had a patch which implemented it.
so to be on the safe side better always check for is_ground_unit...

> Hmm, what do you mean? Player A builds a fortress on a tile on which
> player B's units are? How's this possible? I must admit that I've never

oh, you scared me there.  no it is possible, I just checked.

> >   http://www.freeciv.org/lxr/ident?v=cvs&i=fog_area
> > reveals that you forgot to upgrade unittools.c around line 1541 (unit
> > upgrading).
> 
> Yes, I understand. I'm looking for further flaws in my implementation.

actually I take my words back about line 1541.  Upgrading can only happen
in a city, so you are safe there.  But what about units being killed in
the fortress.  Are you treating this case?


> > 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.
> 
> Up to now, I put them together into get_watchtower_vision(). Then,
> later
> changes will be easier, because only this function needs to be changed.

good strategy

> > 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).
> 
> Well, this was not really my idea, to put anything about the vision
> AREA

err, actually this comment was not addressed to you ;)

I had another comment which I forgot to make:
you should probably modularise your changes.  I mean it's better to have
change_unit_vision function than do it explicitely in all cases.  But it
seems you are doing it anyway now...

Waiting for new version of your patch (but don't hurry),
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