[Freeciv-Dev] Re: Remove map_adjust_[xy] invocations from server (PR#1003)

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

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
> 
> In the short/medium term we can disable (but not remove)
> CHECK_MAP_POS. We will probably leave the check in map_inx forever. So
> I dislike the nearest_real_pos call above. If there are wrong map
> positions we catch them (earlier(CHECK_MAP_POS) or later(map_inx)).

Most of this sounds reasonable.  However, two points:

1.  I do not see the logic of forcing a crash when not in debugging
mode.  Presumably people playing the game in this stage would appreciate
it if we did everything possible to avoid crashes.  For the "extra
check", why not have something like this:

if(!is_normal_map_pos(x, y)) {
  assert(0);
  freelog(...);
  nearest_real_pos(&x, &y);
}

This would give many fewer segfaults (namely: none) for end-user players
of the game.  I'd use this check in place of every CHECK_MAP_POS.  In
fact, this is almost identical to the current check_coords function
(except that there's no assert() there; one could easily be added).

Since nearest_real_pos really is needed for the client (in several
places, all related to moving the view around IIRC), we might as well
use it in this situation.  There's really no way we can do without it
without changing gameplay significantly.


2.  None of this is a reason not to go ahead with this patch.  This
replaces one set of imperfect code with another set, but at least the
new set uses the desired code constructs (i.e. normalize_map_pos instead
of map_adjust_x()).


Either way, I'm willing to come up with a patch that makes the checks we
decide on (although fixing the code to use them everywhere will take
more work).  Let me know what you want.

jason