Complete.Org: Mailing Lists: Archives: freeciv-dev: August 2001:
[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part1 has been put in incoming
Home

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Jason Dorje Short <jshort@xxxxxxxxxxxxx>
Cc: rf13@xxxxxxxxxxxxxxxxxxxxxx, "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>, freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] Corecleanup_07Part1 has been put in incoming
From: "Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>
Date: Sun, 26 Aug 2001 16:12:40 -0400

At 02:51 PM 01/08/26 -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.
>
>jason

You've got it. corecleanup_07f adds the map_wrap and map_trunc functions
and corecleanup_07g uses them in the GUI. Documentation and rationale in 
map.h after 07f is applied.

They were in the earlier versions, but glad to see we are getting around 
to some of the actual enhancements or needed upgrades.

Cheers,
RossW




[Prev in Thread] Current Thread [Next in Thread]