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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Oct 28, 2001 at 06:50:46PM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> "Ross W. Wetmore" wrote:
> > 
> > At 01:19 AM 01/10/24 -0400, Jason Dorje Short wrote:
> > >"Ross W. Wetmore" wrote:
> > >
> > ><snip context: RAND_POS_CHECKED macro>
> > >
> > >> This clearly significantly increases the computation effort,
> > >
> > >Not necessarily.  In a mostly-grassland map the overhead isn't necessary,
> > >but in a highly-forested or highly-desert map the fact that the loops are
> > >bounded would be a big advantage.
> > 
> > You missed the point. The example explaining it is in another email.
> > 
> > BTW: It is bad that you missed the point. It shows you don't understand
> >      the practical aspects/needs.
> 
> I think you missed the counter-point.  Although the "pick-at-random"
> algorithm is many times slower, there is no measurable difference in map
> generation speed while using it.
> 
> > Nothing currently depends on this and nothing needs to depend on this.
> 
> All the rest of the mapgen stuff depends on it.  If we ignore mapgen
> stuff, then things become simpler.
> 
> > BTW: There is no problem running the mapgen.c replacement in any of the
> >      4 rectangular topologies, and in corecleanup_07 the existing mapgen
> >      had minimal fixes to do the same.
> 
> These all have normal==regular.  With an iso-rectangular topology, it
> would quickly segfault.
> 
> > >Your corecleanup patch doesn't touch on mapgen.c so I have no other work to
> > >go by here.  How would you change mapgen to accomplish the above two goals?
> > 
> > They are really more or less there. But if you need more ...
> > 
> > You can apply the mapgen.c replacement that has been in all the 
> > corecleanups.
> > It might even work with CVS if DIR_CCW etc. is used in the places where it
> > does really need to use rotational map_steps. This is pretty much the river
> > code. Some form of block_iterate is also needed. But both of these can be
> > put in as local macro fixes/hacks until the general CVS codebase reaches the
> > necessary level of capability. The initial corecleanup versions actually ran
> > this way, i.e. with self-contined corecleanups.
> 
> Your mapgen code, like all your other corecleanup code, still assumes
> normal==regular and that the map will wrap only along the x and y axes. 
> In fact, you seem to keep forgetting that topologies will not always
> follow this. :-)
> 
> > I would also take responsibility for any bugfixes to keep these standard
> > map generators running through the topology changes, and produce a new
> > generator
> > to handle general topology features agreed upon in some collective proposal.
> > I actually have a strong interest in playing on good maps as opposed to the
> > stuff that is currently produced, so would do some of this on my own anyway.
> 
> This sounds great.  Again, here is my position:
> 
> - I don't want to make the current mapgen code worse.
> - I want mapgen code that is topology-independent, although not all
> generators must be independent.
> - It is quite easy to make the current mapgen code topology-independent
> without impacting current map generation.
> - It may not be so easy to make better mapgen code topology-independent.
> 
> 
> In the meantime, can we agree on this patch?  It replaces all of the
> innocuous random position selection with rand_pos, getting rid of most
> of the "bad" code (including everything not in mapgen).  It does not use
> regular_map_pos_is_normal (which I'd be happy to add back in if
> desired).
> 
> I hope we can bring an end to these simple rand_pos changes, leaving the
> harder ones for later.

I will accept it if you take out the two cases where you use the extra
do-while loop.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "There are three ways to get something done. Do it yourself, hire someone
  to do it for you or forbid your kids to do it."