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

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Raimar Falke wrote:
> 
> On Thu, Oct 18, 2001 at 11:53:08AM -0700, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> > Raimar Falke wrote:
> > >
> > > On Thu, Oct 18, 2001 at 11:13:48AM -0700, jdorje@xxxxxxxxxxxxxxxxxxxxx 
> > > wrote:
> > > > Raimar Falke wrote:
> > > > >
> > > > > On Wed, Oct 17, 2001 at 10:01:24PM -0700, 
> > > > > jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> > > > > > The attached patch creates a function, rand_pos(&x, &y) that 
> > > > > > determines
> > > > > > a random map position.  It also uses it in place of all similar 
> > > > > > code.
> > > >
> > > > > > This could have efficiency problems if we were looking for a very 
> > > > > > small
> > > > > > subset of positions on the map.  Fortunately, there is no such code
> > > > > > right now.
> > > > >
> > > > > I'm also converned about efficiency problems. Can't rand_pos take the
> > > > > range of allowed distances from north. Which now are from
> > > > > 0..map.ysize-1 and latter come from 0..height-1.
> > > >
> > > > This doesn't sound so good.  Outside of map generation stuff (which
> > > > shouldn't be a concern), the only check that's done on the returned
> > > > value is to make sure it's not on one of the poles.  In other words,
> > > > this will attain virtually no speedup except during map generation.
> > > >
> > > > This method also isn't really very general.  For instance, how does it
> > > > apply to a torus map?  A torus map would probably have no polar regions
> > > > (everything would be tropical); this would be easy to achieve using the
> > > > checks I'm proposing but difficult or impossible if you build that
> > > > information into the rand_pos function.
> > > >
> > > > Finnaly, this would make the code much uglier (IMO) and would make
> > > > rand_pos a topology-dependent function, which I'd like to avoid.
> > >
> > > The map generation is topology-dependent. The placement of certain
> > > items (rand_pos is used for this) is topology-dependent. So I have no
> > > problem of having a topology-dependent rand_pos method. This method
> > > has to have a certain knowledge about latitude. But method for this
> > > are needed for the general map creation anyway.
> > >
> > > So what do you think about a general rand_pos method and a map
> > > generation rand_pos method. The second one is local to mapgen.[ch].
> >
> > I don't think map generation has to be particularly topology-dependent,
> > at least not the part you're talking about.
> >
> > BTW: the map iteration patch did not touch mapgen.c because I plan a
> > separate fix for there.
> >
> > I plan a macro DISTANCE_FROM_NORTH_POLE(x,y) which will replace the
> > current use of "y" in most of these places.  Similarly, a macro
> > DISTANCE_FROM_SOUTH_POLE(x,y) can replace the current use of
> 
> The DISTANCE_FROM_SOUTH_POLE isn't needed since
> DISTANCE_FROM_SOUTH_POLE = height - DISTANCE_FROM_NORTH_POLE. But
> you may introduce a DISTANCE_FROM_WEST_POLE.

Not necessarily.  You're thinking linearly, whereas not all topologies
may work this way.  For instance, an ellipse may have positions on the
side be closer to the poles than positions in the interior that are at
the same altitude, in this way getting a polar cover at more than just
the top and bottom rows of the map.  This will lead to the distances not
being continuous.

More generally, on an actual sphere the distances from north and south
poles always add up.  But on a differently shaped world, they need not. 
In this context, TOTAL_POLAR_DISTANCE() is just a scale factor.

Or, to put it yet another way, we're usually only concerned with the
distance to the nearest pole, which is much easier to compute for most
maps and is what we're interested in.  Since this nearest pole may be
either north or south, it might be easier to have code in both macros to
compute distance.

Or, to put it yet another way, we may end up defining
DISTANCE_FROM_SOUTH_POLE as
TOTAL_POLAR_DISTANCE()-DISTANCE_FROM_NORTH_POLE(x,y).

> > "map.ysize-1-y" (which is sometimes incorrectly referred to as
> > "map.ysize-y", I believe).  Instead of using map.ysize in the
> > calculations, we'll use TOTAL_POLAR_DISTANCE().  Introducing a fourth
> > macro, DISTANCE_FROM_POLE(x,y) to be the minimum of the distance from
> > the north and south pole allows lots of the code to be made much
> > cleaner.
> >
> > These macros (well, all but the fourth one) are topology-dependent, but
> > they're pretty easy to write for most topologies.  Using them will make
> > it pretty easy for specific topologies to have different types of maps
> > under the same mapgen code: for instance a torus map would probably not
> > have any poles, so would always have
> > DISTANCE_FROM_POLE(x,y)==TOTAL_POLAR_DISTANCE()/2, making everything
> > tropical.  (This idea was proposed by Reinier; a torus map done this way
> > makes things fairer in multiplayer mode.)  It is not easily achievable
> > if things are done as you propose.
> >
> > Ultimately, there may be more problems with the mapgen code and some map
> > generators may not work for specific topologies.  This will be a deeper
> > problem, though, and won't have such a simple solution.
> 
> I think the idea is good but need integration into the (to be created)
> framework. So if there is a map.topology.height there should be a
> get_height_of_map_pos(x,y) which has a range of
> [0..map.topology.height-1]. Same for width. Or an
> transform_to_xyz_system(&width,&height,x,y). The question is: how is
> the new system named?

This makes sense.  But, map.topology.height might not equate to
TOTAL_POLAR_DISTANCE().  Then again, it might.  It depends on how we
implement it.

Another problem is this forces map generation code to assume the poles
are located at height==0 and height==map.topology.height-1, which may
not be the case.  Having DISTANCE_FROM_***_POLE as a wrapper still
allows us to use this sort of system as a backend.

So, in summary using functions/macros to access all this data gives us a
lot of flexibility.  But we still need to decide what form the backend
accesses should take.


Here's another question: there's really two different checks here,
DISTANCE_FROM_POLE and DISTANCE_FROM_EDGE.  An "edge" is the edge of the
map, where it does not wrap around.  Under the current topology
DISTANCE_FROM_POLE(x,y)==DISTANCE_FROM_EDGE(x,y).  The question is,
which is intended for each check?

Obviously for making arctic regions it is the DISTANCE_FROM_POLE we're
interested in.  But for make_passable() it might be DISTANCE_FROM_EDGE. 
I'm really not sure what's best here.  Running make_passable() on an
elliptic map if it uses DISTANCE_FROM_POLE() really wouldn't make all
that much sense.

jason