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

["rwetmore@xxxxxxxxxxxx via RT" <rt@xxxxxxxxxxxxxx>]

I think you are running into the historical problems here :-).

Now that there are a number of maintainers, it should be the case that
those maintainers that do not understand and aren't willing to put in
the effort should simply abstain from the discussion. The one or two
that actually do take the time should be sufficient to vet the code.

I agree with GB's comments. The code is apart from the DIVIDE ugliness
is reasonable. It would be nice to simplify this using the the current
known constraints to get rid of the % operations. The commentary seems
to be the source of much confusion. Simple algorithms have less noise
to generate controversy.

But basically, it should get into CVS.

Cheers,
RossW
=====

At 10:02 AM 02/11/14 -0800, Jason Short via RT wrote:
>
>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