[Freeciv-Dev] (PR#8632) Easy way to set map size with auto ratios (seteables if desired)

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8632 >

> [mburda - Thu May 27 14:22:24 2004]:

> -fixed MAP_HEIGHT, there are right now?

No.

  nat(0,0) = map(0, map.xsize)
  nat(xsize-1, ysize-1) = map(ysize/2 + xsize - 1, ysize/2)
  nat(0, ysize-1) = map(ysize/2, ysize/2 + xsize - 1)
  nat(xsize-1, 0) = (xsize-1, 1)

This can be verified in a just a few minutes by plugging the values into
the functions.  It should probably be added to a comment somewhere.  Of
course it's possible I made a mistake (the "1" in the last line is
surprising and disturbing - I don't remember this).

so MAP_WIDTH and MAP_HEIGHT are the same.  Both are

  ysize/2 + xsize

which makes sense when you consider that any rectangle rotated pi/4 has
a square for its bounding box.

> -i am a litle change LOG message, now we send a warning message when
> size was to big.

Hmm, ok.

I'm not happy with how you set size=0 before calling map_init_topology
as a "flag" not to recalculate xsize/ysize.  Instead I think you should
pass an is_new_game boolean parameter to the function.


The info for the topology server variable is confusing.  What does [52]
mean?  I think this info should be moved into the ratio variable, since
the user probably shouldn't care about it most of the time.  And instead
of [52] say 5:2 (or [5:2]).


The other things are pretty small...


You have
   mcd = gcf(Xratio, Yratio);
which is inconsistent (we need just 1 naming system).


In one place you have

+    mcd = 0; /* != 1 force to correct map.ratio if this is not a auto
ratio */
+  }
+  /* set corrected ratio in map.ratio if these values are set by user */
+  if( map.ratio != 100 && mcd != 1) {
+    map.ratio = 10 * Xratio + Yratio;
+    freelog(LOG_NORMAL,
+       "map.c: The user defined ratios was corrected to %d \n", map.ratio);
+  }

First if you use LOG_NORMAL you need to have a more comprehensible
message (it should NOT mention the filename) and it needs to be
translated.  This is not an ERROR so you don't need any higher log
level, but you could use LOG_VERBOSE/LOG_DEBUG if you want.

Second the mcd != 1 check is  ugly because you have to have mcd = 0 in
the line above to make it work.  How about instead

  int newratio = 10 * Xratio + Yratio;
  if (newratio != map.ratio) {
    /* ... */
  }

This is a small thing but the mcd = 0 line isn't good.


Later on you have

+  /* set map.[xy]size as even numbers (and ysize %4 == 0 for TF_ISO)*/
+  map.xsize =       2 * Xratio * i_size;
+  map.ysize = iso * 2 * Yratio * i_size;

but why are even numbers required?  The only requirement AFAIK is that
ysize must be even for iso-maps.


Then you have

+  if (MAX(MAP_WIDTH,MAP_HEIGHT) - 1 > MAP_MAX_LINEAR_SIZE ) {
+      /* make a more litle map if possible */
+    assert(base_size > 0.1);
+    set_ratio(base_size - 0.1, Xratio, Yratio);
+    return;
+  }

but I don't think we should have any recursive calls to set_ratio. 
However this is tough to fix any other way.

And finally can you add an

  assert(TF_WRAPX | TF_WRAPY == 0x3);

into map_init_topology.

jason