[Freeciv-Dev] Re: PATCH: rand_pos function and usage(PR#1017)

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

At 06:43 PM 01/10/28 -0500, Jason Dorje Short wrote:
>"Ross W. Wetmore" wrote:
>> 
>> rand_pos() is fine, as the normal condition should be a noop for
>> all current map topologies, and (hopefully) all but a small number
>> of positions for any forseeable properly implemented ones, i.e.
>> this while loop is expected to run once on average.
>
>Ahhh...this is exactly what I wanted to hear (well, part of it anyway)
>:-).
>
[...]
>If we make an alternate rand_pos that is topology-dependent, we can pass
>in the range of heights that we want.  This should give us a clean
>interface and be very fast (constant time for most topologies; some may
>be tricky).  I dislike this method because it makes rand_pos
>topology-dependent (which it really doesn't have to be), and doesn't
>allow as much flexibility as the above-mentioned forms (although it is
>sufficient for the current code's needs, and some of the above forms can
>be used in any case).

Don't even think about it. Use rand_pos() for the generic xsize*ysize 
whole_map case, and leave all the code that selects subsets of the map 
for random positions to select as they need. Leave the responsibility 
for coding each of these cases to the code that does it, i.e. hands off.

This all works fine, doesn't fault or cause any problems with all
rectangular topologies which is what this code is specialized to handle.
Nothing needs fixing here at this point.

The changes you are making aren't necessary, really screw things up
and in horribly limiting and inefficient ways, and are frankly poorly
thought through or programmed. As such they don't pass any minimal
standards to get into CVS, and so until there is something that does
leave things as is.

>Like I said, I really don't care what form(s) rand_pos takes.  But I
>would like to extract the topology-dependent code from mapgen and other
>places, and this requires making a separate function that does this.

No. Good maps are *not* topology independent, but really understand a
lot about the characteristics of the underlying topology. The idea that
you can somehow remove all this without totally lobotomizing the output
is quite frankly overzealous foolishness. Anyone who thinks this way
should not be touching this code.

The way to go with map generation is more map generators and absolutely
minimal local changes to fix specific real faults as they occur in 
cases where you might be trying to extend a particular generator to
do something minimal but safe for cases it doesn't really apply to. And
this is not *all* casee, but only limited extensions where such changes
are minor and non-intrusive. 

Everything should *not* work everywhere, i.e. be constrained to some
unwieldy, inefficient and grossly limiting bland common denominator.

>jason

Cheers,
RossW
=====