[Freeciv-Dev] Re: (PR#1180) [PATCH] cleanup to canvas_pos_to_map_pos

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

Raimar Falke via RT wrote:
> On Wed, Nov 13, 2002 at 03:36:24PM -0800, Jason Short via RT wrote:
> 
>>Is there any chance of any of these variants of the patch making it in
>>to CVS?  If not, why not, and what needs to be done?
> 
> 
> I didn't understand the first version by looking at it shortly. I
> don't undestand your version by looking at it shortly. It looks to me
> that this can't be expressed in an easy way where you can comprehens
> it easy.

Ahh...

It is a slightly complicated issue, yes.  I believe the goal should be 
to make it understandable to future readers.

To begin with, don't read the patch as a patch.  IMO the old code is 
almost entirely incomprehensible, and trying to comare the two is doomed 
to failure.  Instead you should look at the code itself once the patch 
has been applied.  You may look at the old code as well.  But don't 
focus on whether or not the two pieces of code do the same thing - 
again, this is nearly impossible to determine.  Try to see why each 
separate implementation is correct on its own.

Then if you find the old code more correct/understandable, then 
obviously I'm doing something wrong.  But I'm pretty confident that 
won't happen :-).  And if you don't understand the new code, then why 
not?  What more can be said to make the explanation clearer?

The transformation used here is at the heart of the use of isometric 
view.  You may also want to take a look at map_pos_to_canvas_pos, which 
does the opposite transformation (it is a bit simpler).  There are also 
corresponding function city_pos_to_canvas_pos/canvas_pos_to_city_pos, 
which are special cases of this conversion (and thus also simpler).

jason