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

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

"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)
:-).

> Ditch RAND_POS_CHECKED (and any places where you tried to apply it)
> since as coded it does map.xsize*map.ysize tests to return a single
> result. You clearly don't understand what you are doing here, so
> should not touch the current code until you have figured out
> something sensible to do :-).

I understand that it is inefficient.  I also understand that the simple
rand_pos inside a loop is less inefficient, but uglier and unbounded (if
the loop is poorly written).  Finally, I understand that none of this
has a significant impact on map generation speed, much less overall
execution speed (I've tested it).

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).

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.

jason