[Freeciv-Dev] Re: [PATCH] Map cleanups (PR#1208)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 10:22 AM 02/01/08 +0100, Raimar Falke wrote:
>On Tue, Jan 08, 2002 at 12:26:00AM -0500, Ross W. Wetmore wrote:
>> 
>> I actually like something like this better than nearest_real_pos(). I am
>> assuming when it is expressed correctly it effectively returns the original
>> unstepped coordinates. More importantly, it should clearly document that 
>> this is what it does as part of the interface.
>> 
>> The case where one steps in a loop until move points are exhausted, and
>> hits an unreal tile along the way (causing a serious slowdown in game
>> response :-) is at least indirectly covered by this warning/documentation
>> then.
>> 
>> At 05:23 AM 02/01/07 -0500, Jason Short wrote:
>> >jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>> >
>> >
>> >> I do think SAFE_MAPSTEP is wrongly implemented.  It needn't use 
>> >> nearest_real_pos but should instead do
>> >>   #define SAFE_MAPSTEP(x, y)
>> >>   {
>> >>     int _x = x, _y = y;
>> >>     if (!normalize_map_pos(&x, &y)) {
>> >>       x = _x;
>> >>       y = _y;
>> >>     }
>> >>   }
>> >
>> >Oops - I forgot I snuck this in in the most recent tilespec fix.  So 
>> >scratch that idea :-).
>> >
>> >jason
>> 
>> But if you look closely ...
>> 
>> In fact what you have done here is one of the many local variants of
>> normalize_map_pos() that are going to spring up because you didn't 
>> make the condition we all say is desired, i.e. the one used here, part
>> of the interface, and thus force people to implement this privately all 
>> over the codebase.
>> 
>> It is better to just do it. Think of it as a code reduction measure, 
>> i.e. of all the duplicate code you are saving by moving it into the
>> implementation from just outside :-).
>> 
>> If someone really wants arbitrary values returned, and the void_tile
>> sentiments revive to carry the day to give this a new lease on life, 
>> then it is possible to expose the underlying internal interface in 
>> normalize_map_pos(), i.e. below the is_real check and let them 
>> experience the Chinese curse of "may you live in interesting times".
>> 
>> This way at least code that uses the primary interface won't suffer.
>> And you will have to go slightly out of your way to do the stupid
>> thing. It is always wise to make the defaults safe, and not the 
>> "special" case :-).
>
>Ross I completely agree with you. 
>
>*trumpets* A historical date. *trumpets*

It should always be fairly easy to repeat this occasion if we try to
stick to technical issues in our disagreements.

If we did this for awhile until it became habit, we could probably widen
the scope of interaction occasionally too :-).

>       Raimar
>-- 
> email: rf13@xxxxxxxxxxxxxxxxx
>  Two OS engineers facing a petri net chart:
>        "dead lock in four moves!"

Cheers,
RossW
=====