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

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

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
> >
> > 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)).

...

> 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.

Upon further reflection, I really like the following change that unifies
check_coords with CHECK_MAP_POS.

Though we may want to rename the function check_map_pos().  It could
also be made a macro, if desired.


/**************************************************************************
-...
+This used to return an integer value on whether the position was real
or
+not.  However, its "new" use is basically to assert that the position
is
+normal.  If not, it will abort when in debug mode and fix the position
+otherwise.  Thus, there is no longer a need for a return value, since
the
+coordinates will be usable either way.

**************************************************************************/
-int check_coords(int *x, int *y)
+void check_coords(int *x, int *y)
 {
   int save_x = *x, save_y = *y;
 
-  if (!normalize_map_pos(x, y)) {
-    freelog(LOG_ERROR, "%d, %d is not a real tile!", *x, *y);
+  if (!normalize_map_pos(x, y) || *x != save_x || *y != save_y) {
     nearest_real_pos(x, y);
-    return 0;
+    freelog(LOG_ERROR, "(%d, %d) is not a proper position.  Adjusted to
(%d, %d)!", save_x, save_y, *x, *y);
+    assert(0);
   }
-
-  if (*x != save_x || *y != save_y)
-    freelog(LOG_ERROR, "Had to adjust coords %d, %d", *x, *y);
-
-  return 1;
 }

(Though actually the code itself is unoptimized, but that's easily
fixed.)

jason