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

["Marcelo Burda" <mburda@xxxxxxxxx>]

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

Le ven 28/05/2004 à 07:06, Jason Short a écrit :
> <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]).
> 
at end of list i was put "[] maximal size". but probably the warnig of
too big size is ok.
> 
> The other things are pretty small...
> 
> 
> You have
>    mcd = gcf(Xratio, Yratio);
> which is inconsistent (we need just 1 naming system).
> 
hmm.. ok ok

> 
> 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.
> 
ok
> 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.
> 
hmm.. :-S ( ;-) )
> 
> 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.
that is not realy a problem to force event numbers today, then new
scenario and savegames will be make in even number too, this is needed
for newest topologies as quincuntial or twisted torus(a torus rotated
45°)
but this is needed too if you want make a iso overview from a not iso
map. choicing a optimized view  for torus (i will be explain this in an
other patch) 
> 
> 
> 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.
?
Why not? this is a rarely used code, we not need optimizing it and this
way is a absolutly clear one.
> 
> And finally can you add an
> 
>   assert(TF_WRAPX | TF_WRAPY == 0x3);
absolutly unided! and TF_WRAPY == 0x3 is only for iso. but if you realy
like it.
> 
> into map_init_topology.
> 
> jason
-- 
 . /  .     '    ,    .      (*)   '        `     '      `    .    
  |    ,  |   `     ,     .      ,   '  Marcelo Julián Burda      .
 /  '     \     `     \@_     '      .        '      `        '    
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~