[Freeciv-Dev] Re: PATCH: map iteration (PR#1018)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Oct 30, 2001 at 08:24:45PM -0500, Ross W. Wetmore wrote:
> At 05:17 AM 01/10/30 -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> >3.  Is the debugging function print_landarea_map() in game.c (part of
> >the LAND_AREA_DEBUG group) necessary?  If no consensus can be reached, I
> >suppose it must be left in.
> 
> Someone removed a lot of these from mapgen at one time. I put new ones
> back when I need to work in the code. There's probably a lesson in there
> somewhere ...

The debug output in game.c prints the claim_map which is used for the
score calculation. I don't see how this relates to mapgen. But I agree
that the claim calculation is complex and may needs to be debugged
some time. Can we cleanup the code? Replace printf with freelog? 
Remove the #ifs from get_player_landarea?

> >4.  Is regular_map_pos_is_normal good, or should we stick to
> >is_normal_map_pos?  Again, regular_map_pos_is_normal provides identical
> >functionality but assumes the map position it is given is "regular",
> >i.e. within (0..map.xsize-1,0..map.ysize-1).  This makes it much faster.
> 
> I'm not much of a fan of regular. You need to demonstrate it a bit
> more to show any real value. If it never does anything, then dubious
> dregs like this are just noise.
> 

> Another maxim that comes to mind is premature optimization is the root
> of all evil. I might be best to wait until you have demonstrated there
> is a problem before optimizing. That shouldn't take too long, though.

I agree. However we already have code in there which is a noop for the
current topology.

> >6.  I have not touched any mapgen stuff, although some of the loops
> >there could easily use whole_map_iterate.  Is this desired?
> 
> Raimar needs to decide what level of activity he wants here.

begin quote
> > ./server/mapgen.c:111:    for (x=0;x<map.xsize;x++) {
> > ./server/mapgen.c:123:    for (x=0;x<map.xsize;x++) {
> > ./server/mapgen.c:138:  for (x=0;x<map.xsize;x++) {
> > ./server/mapgen.c:724:      for (i = 0; i < map.xsize * map.ysize; i++) {
> > ./server/mapgen.c:782:  for (x=0;x<map.xsize;x++) {
> > ./server/mapgen.c:801:    for (x=0;x<map.xsize;x++) {
> > ./server/mapgen.c:1406:    for (x=0;x<map.xsize; x++) {
> > ./server/mapgen.c:1765:    for (x = 0 ; x < map.xsize ; x++) {
> > ./server/mapgen.c:1769:  for (x = 0 ; x < map.xsize; x++) {
> >
> > Some of them can be replaced with a new rectangle/block_iterate.
>
> I think all of them can, and the new code should be cleaner as well.
> There are other issues with converting this stuff, as discussed in the
> other thread.
end quote

> >7.  I have also not touched Overview_FillBuffer in gui-mui.  I don't
> >know what the correct fix there might be.
> >
> >Once these issues are resolved, we should be able to think about closing
> >this.

We don't have to convert every for (x = 0; x < map.xsize; x++) loop to
something other.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Premature optimization is the root of all evil."
    -- D. E. Knuth in "Structured Programming with go to Statements"