[Freeciv-Dev] Re: Remove map_adjust_[xy] invocations from server (PR#1003)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Oct 22, 2001 at 02:47:49AM +0100, Gaute B Strokkenes wrote:
> On Sat, 13 Oct 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> 
> > But in the long term we want normalized map positions and the code
> > IS ready to 99%. So we should go IMHO the full way and just say that
> > function arguements have to be normalized and deal with rest (~1%)
> > in the next weeks and be done with it.
> 
> Now, that's the most sensible thing that anyone has said on this
> subject in the last week or so.
> 
> Before one falls over one self trying to work out clever ways to come
> up with functions/macros/spells that catch dodgy coordinates and then
> either abort or try to fix the situation, it is worth remembering that
> pretty much the only way that you will ever see one of these is if
> you manually do what one of the _iterate macros do, not use MAPSTEP
> where you should or if you do something like
> 
>    map_get_tile(pcity->x +cx-2, pcity->y + cy-2).
> 
> We have removed pretty much all such constructs from the code base,
> and unless someone does something really stupid we are unlikely to see
> much more of them.
> 
> This is why I think the best thing would be to add this sort of check
> to map_inx().  That way, we:
> 
>   * Get an easy way to turn on comprehensive checking for unnormalised
>     coordinates if you are working on changes to that part of the
>     code.
> 
>   * Avoid splattering explicit checks all over the code.
> 
>   * Avoid the need to explicitly add a check in every single place we
>     use a coordinate pair, or pass it to another function.
> 
> The flip side is that you have to add an explicit check to places
> where you do more complicated stuff, e.g. same_pos().  Off course, if
> you wanted an explicit test everywhere then you would have to add one
> there anyway.

I agree to this solution for all positions are at some time used as
arguments to map_get_tile and co.  The question is: what happends to
the rest? Either we manually go through the code and set
assert(is_normal_map_pos()) (which is error prone and not robust) OR
we ignore this case OR we take out the big hammer and insert
assert(is_normal_map_pos()) into almost every function and disable it
by default (because this will have a performance impact).

I don't know how many of the map positions are never used as an
argument to map_get_tile and co. But I'm sure it is under 5%.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "SIGDANGER - The System is likely to crash soon"