[Freeciv-Dev] Re: Map coordinate cleanups.

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Wed, 15 Aug 2001, xyzzy@xxxxxxxxxxxxx wrote:
> On Thu, 16 Aug 2001, Gaute B Strokkenes wrote:
>> Attached is patch that fixes several instances where Freeciv
>> inadvertently creates unnormalised map coordinates or makes use of
>> unreal map coordinates.  It also changes the is_real_tile()
>> function to a macro IS_REAL_TILE(), and adds macros IS_NORM_TILE()
>> and IS_PROP_TILE(), and some other coordinate related things.
> 
> If you ever want to support topologies other than a cylinder, you
> probably don't want to make these a macro.  IS_NORM_TILE would
> become quite a mess of ?: operators

#define IN_RANGE(value,range) (((unsigned) (val)) < ((unsigned) (range)))

#define IS_NORMAL_TILE(x,y)                    \
  ((map.ywrap && IN_RANGE((y), map.ysize))     \
   && (map.xwrap && IN_RANGE((x), map.xsize)))

> just to support the three most simple map types of plane, cylinder,
> and sphere,

I think you mean torus here.

> not even considering the crazy shit like Mobius strips some people
> were talking about.

That's completely equivalent to a cylinder for the purposes for these
macros; a Klein bottle is equivalent to a torus.

If we ever have to change this back (which I don't think we have to,
until and unless we implement `isometric view' shapes), it's only
going to take two minutes to change it back, anyway.

> Also, most of the places these functions are used is inside
> debugging assert calls.  There is no point in making those faster
> since they are just for debugging.  So there is no good reason to
> make it a macro in the first place.

That's not true.  Compiling with assertions disabled is the exception
rather than the norm, so most people would benefit from a speedup.
Moreover, it is used in several places where we're looping over
geographical regions and the like, and it would be good to locally
expose the nature of this test to the compiler.

In any case, some time ago there was a discussion about using inline
functions for this sort of thing.  It petered out because of various
concerns about portability and / or performance which no one could
summon the energy to investigate properly.  If someone could be
bothered to search the list archives and investigate this properly we
might be able to conclude that we no longer need any macros at all.

> I also wonder if making city_map_iterate use normalized map
> coordinates, or creating another version that does, might be a good
> idea.  Is there much (any?) code that wants to iterate over
> non-normalized coordinates in the city map?

> I also don't think making normal no longer a subset of real makes
> much sense.

I haven't redefined anything; there is no evidence anywhere in the
current sources that this has been given any thought at all.  In any
case my point is that

  Stepping of the edge of the world.

and

  Accidentally forgetting to wrap an x coordinate.

are DIFFERENT kinds of error, and accordingly you want to be able to
detect either kind or both according to your needs.

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
Let me do my TRIBUTE to FISHNET STOCKINGS...