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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Oct 19, 2001 at 07:16:42AM -0400, Jason Dorje Short wrote:
> 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.

I know that such distances aren't available on all topologies. However
the non-iso rectangle and the iso-rectangle has such distances. And
yes I would also prefer to have a distance_from_[nwse]_edge. This
would be the base where other stuff like distance_from_[ns]_pole can
be defined. I also agree that map.topology.height is more a
map.topology.max_height.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  What's nice about GUI is that you see what you manipulate.
  What's bad about GUI is that you can only manipulate what you see.