[Freeciv-Dev] Re: normalize_map_pos and invalid map positions

[Jason Short <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Raimar Falke wrote:
> On Thu, Aug 15, 2002 at 11:43:39AM -0500, Jason Short wrote:
>
> > Per I. Mathisen wrote:
> > So what it comes down to is three different concerns: speed, safety, and 
> > error-catching.  And it seems everyone values each concern differently 
> > (I put the most emphasis on error-catching).
>
> We didn't catch a single error with CHECK_MAP_POS. So it was
> disabled. However I have no problem to reenabling it if we work heavy
> on the map.
We caught several existing errors with CHECK_MAP_POS - for instance air 
gotos were not getting normalized properly, and failed the CHECK_MAP_POS 
check.  The last such segfault happened months ago.  There is a high 
likelihood there are no more such bugs in the current code.
But new code is written all the time, and CHECK_MAP_POS is needed to 
help detect bugs there.  For instance, Stewart's borders patch initially 
did not correctly use normalize_map_pos; this caused a CHECK_MAP_POS to 
fail and the bug was easily spotted.  If a similar mistake was made 
again now it would take longer to detect since most people don't compile 
with DEBUG.
I agree with you that it can be changed at any time.  But I do think 
that there's a big advantage to be gotten from having it enabled so that 
the majority of people who play with development code (including normal 
players who play the CVS version and don't know anything about DEBUG vs 
NDEBUG) can help us to debug just by playing.
> > In theory the set of "normal" positions could include (-1,-1).  I think 
> > we all agree that it should not, though.
>
> Yes. Any normal position is a regular position (see
> common/map.h:regular_map_pos_is_normal). So we have
>
> real positions are a (proper) subset of all map positions
>   are equal if wrapping in both axises is enabled
> regualr positions are a (proper) subset of real positions
>   are equal if no wrapping is enabled
> normal positions are a (proper) subset of regualr positions
>   are equal in "classic" (non-iso) maps
"regular" positions are not, in general, a subset of real positions 
(consider for instance an unwrapping iso-rectangular map).  Thus normal 
positions are a subset of both regular and real positions.
With the discovery of native/compressed coordinate systems, most uses of 
regular positions are no longer needed.
> > Most of this can be seen in action in Ross's corecleanups.  He has a 
> > clean system of converting from one type of coordinate to another - 
> > map_to_native_pos, native_to_map_pos, etc.  But when these things try to 
> > make their way into CVS, we always run into minor stumbling blocks that 
> > hold things up indefinitely (like the speed vs safety vs error-catching 
> > issue above).
>
> And this is the part where I lost Jason and Ross the last time. In
> addition to the three sets of map positions we now get at least two
> representations.
It is confusing, that we have both *sets* of positions as well as 
*representations*.  One thing we haven't done well is clarify this 
distinction; a statement like "all native coordinates are normal" is 
very misleading.
The corecleanups are hard to read since they change so much code. 
However, a change like this is pretty significant and will change a fair 
amount of code...eventually.
jason