[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part1 has been put in incoming

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Raimar Falke wrote:
> 
> On Sun, Aug 26, 2001 at 10:50:00AM -0400, Ross W. Wetmore wrote:

> > I assure you that without most of those changes it is easy to generate
> > core files. And things like added comments were requested.
> 
> As I said: before we add comments like:
> -  dest_x = map_adjust_x(x0+dx);
> -  dest_y = map_adjust_y(y0+dy);
> +  /* TODO: if this returns false, tile is bad. Shouldn't we just return? */
> +  dest_x = (x0+dx);
> +  dest_y = (y0+dy);
> +  normalize_map_pos(&dest_x,&dest_y);
> 
> Can't we just declare this methods needs real tiles and this method
> doesn't?! Or at least make it an assert so that this comes up later. I
> just fear that not many people read comments. There are 126 fixmes and
> 19 todos in the source.

Now seems like a good time to point out that with code like this we have
a problem.

In may places x=map_adjust_x(x), y=map_adjust_y(y) is used blindly, and
really should be normalize_map_pos instead (which Ross has done). 
However there are a few places in the GUI where y=map_adjust_y is used
intentionally.  For instance, certain mouse clicks that are "off the
map" have their values translated to the closest point on the map.  This
needs another solution: either a new map translation function, a changed
(more explicit definition) of normalize_map_pos, or a change so that the
behavior is no longer necessary at all.

jason