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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Aug 26, 2001 at 02:51:29PM -0400, Jason Dorje Short wrote:
> 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.

How does this depend on the "showing a tile multiple time" thing?

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  The Software is not designed or licensed for use in on-line control
  equipment in hazardous environments, such as operation of nuclear
  facilities, aircraft navigation or control, or direct life support
  machines. 
    -- Java Compiler Compiler Download and License Agreement