[Freeciv-Dev] Re: Remove map_adjust_[xy] invocationsfromserver (PR#1003)

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

"Ross W. Wetmore" wrote:
> 
> At 12:53 AM 01/10/13 -0400, Jason Dorje Short wrote:
> >"Ross W. Wetmore" wrote:
> >> At 05:18 AM 01/10/12 -0400, Jason Dorje Short wrote:
> >> >Raimar Falke wrote:
> >> >[snip: uses of is_real_tile, normalize_map_pos, etc.]
> >> >
> >> >> I plan to propose something like this:
> >> >>  - there is a CHECK_MAP_POS(x,y) == assert(is_normal_map_pos(x,y));
> >> >>  - every "assert(is_real_tile(x,y));normalize_map_pos(x,y)" is
> >> >>  replaced by CHECK_MAP_POS
> >> >>  - there is an extra check (if(!is_normal_map_pos(x,y)){y=x/0;})
> >> >>  introduced in map_inx
> >>
> >> As far as I am aware, is_normal_map_pos() is not the same thing as
> >> is_real_tile(). It tests for normalized and not realness which are
> >> different concepts that you really need to keep straight.
> >
> >Correct.  What Raimar is proposing is not a substitution of a macro for
> >identical code, but rather "fixing" the code to always pass normal
> >coordinates.
> 
> The code really isn't ready for this. You haven't yet added all the fixes
> for the case of guaranteed realness, let alone normalized.
> 
> And there may be perfectly valid reasons for passing around (real but)
> unnormalized coordinates and doing the normalization as the last step
> and not the first. Gaute's examples of the "obvious way to do things"
> and the results should serve as warnings to all here :-).
> 
> In the meantime, you are probably just going to break a lot of things
> unless you do it carefully one step at a time.

The idea is to break things without doing so fatally.  My preferred
CHECK_MAP_POS macro does this:

int check_map_pos(int *x, int *y)
{
  if (is_normal_map_pos(*x, *y)) return;
  assert(0);
  freelog(LOG_ERROR, ...);
  if (!normalize_map_pos(x, y)) {
    freelog(LOG_ERROR, ...)
    nearest_real_pos(x, y);
  }
}

This code will be identical to the current code when in NDEBUG, and the
only difference when in DEBUG is that it asserts when bad coordinates
are found.  This will let us easily find the problem areas.  Note: there
aren't very many problem areas; in the substitutions I've done I have
yet to see a single failure.

jason