[Freeciv-Dev] Re: [PATCH] is_normal_tile function

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 01:31 AM 01/08/16 +0200, Gaute B Strokkenes wrote:
>On Wed, 15 Aug 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
>> On Wed, Aug 15, 2001 at 09:58:37PM +0100, Gregory Berkolaiko wrote:
>>>  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: > On Wed,
>>>      Aug 15,
>>> 2001 at 09:41:17PM +0100, Gregory Berkolaiko wrote:
>>> > >  --- Jason Dorje Short <jshort@xxxxxxxxxxxxx> wrote: 
>>> > > > +***************************************************************/
>>> > > > +int is_normal_tile(int x, int y) { return 0 <= y && y <
>>> > > > +map.ysize && 0 <= x && x < map.xsize;
>>> > > > +}
[...]
>I have a number of issues with Ross' patch, but this isn't one of
>them.  My reasoning is that although it is ugly, it is ugliness that
>is easily fenced off, given a name and then explained in a comment.
>Off course, it's not worth it unless you put it in a macro anyway.  In
>that case, it would also help to avoid issues with side-effects of y.
>
>Also, this patch is IMHO wrong.  A tile such as (4, -1) is, in the way
>that I see things, normalised but not real.  A better name for this
>function would be is_proper_tile().
>
>-- 
>Big Gaute                               http://www.srcf.ucam.org/~gs234/
>Sign my PETITION.

I think it is going to help to separate label and concept with this one.

I believe that everyone can agree that the current code defines
  is_real_tile() 
to mean that the tile is "allowed" or can be "normalized" and it is often
found in asserts followed by a normalization.

There is a need for checking for a "normal_tile" that has already been
"normalized" and is within the (storage or whatever) range.

Now you can reverse the names if you like, but lets keep the underlying
concepts and the applicability correct, or chaos is likely to ensue :-).


As an aside, wrapped coordinates are always "real", as they can be 
normalized to a "real" object. Non-wrapped coordinates as in Flat-Earth
or in the "Y" direction in the standard map are where the problems come 
from, as a non-real tile position can never be legitimately "normalized".

Luckily, Freeciv doesn't do a lot in the polar regions, but when you run
Flat-Earth, you can turn up lots of manually coded instances of bad
reality checks or unchecked hand iteration.

Cheers,
RossW