[Freeciv-Dev] Re: map_adjust_x (was: Profiling Civserver again)

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Mike Kaufman wrote:
> [someone unknown (Thue?) wrote:]
> > True, you could check all uses. But to limit map_adjust_x like this
> > would just make it into a minefield for programmers. Then rather have
> > freeciv run a little bit slower IMO.
> 
> I can agree with the possible end result: if the general consensus is 
> that safer is better, then no problem. But I don't agree with the 
> particular reasoning. map_adjust_x() is like any other function or macro. If 
> a programmer wants to use it, then he had better know what it's going to do. 
> If that takes documenting the code, so be it. map_adjust_x() is not a 
> safety net. It ought to exist primarily because our game happens to wrap 
> around in the x direction. Input, by design, will go a bit out of bounds.

I think this would be an ideal time to make a point I've noticed
recently.

Under an arbitrary wrapping scheme, map_adjust_x and map_adjust_y will
not work.  The X and Y coordinates must be normalized together. 
Consider the isometric map mentioned recently - you *can't* wrap X
without knowing Y.  Just a situation in which there were three different
possible wrapping types would make using map_adjust_x and map_adjust_y
macros for this very ugly.

My conclusion?  We should stop wasting time worrying about map_adjust_x
(although it is interesting discussion).  After Monday (or whenever
1.12.0 is released), a campaign to stomp out the usage of map_adjust_x
and map_adjust_y should be instituted.  They should all be replaced by
normalize_map_pos.  No piece of code should wrap manually.  The
adjc_***_iterate macros should use a function is_border_tile (or
is_border_map_pos) to determine whether normalization is necessary, and
use normalize_map_pos in that situation.

jason