[Freeciv-Dev] (PR#2749) place_land_type in mapgen

["Gregory Berkolaiko via RT" <rt@xxxxxxxxxxxxxx>]

I like the patch a lot.  But have plenty of nit- or so picks :

1. Why you don't like calloc?

2. You should check in place_land_type (which is really place_ter_type) if 
the function returns value within the bounds.  "We require" put in the 
comment doesn't always help.

3. Placing mountains and hills depending not only on height but on 
gradient too is a very noble aim, but maybe a bit excessive at this time 
(this is directed to Ross too, who I believe started it in his cleanups).
I seem to recollect how someone was arguing against artillery bombarding 
ships: "the scale is wrong".  I feel similar arguments are applicable 
here.  Even if they aren't, we can still use the height as the first 
approximation of whether it's a hill or plains.

4. In tile_max_steepness, slope is initialized in an ugly way.  Maybe can 
use -maxval or even -FC_INFINITY

5. Recursive placing of deserts was removed.  But you can do it even in 
you framework (and then do forests similarly).

6. The style of comments: they shouldn't have "*" in the end of the lines.

On Sun, 5 Jan 2003, Karen Yeats wrote:

> I have made one incidental change in mapgen for my convenience, namely the
> world is now plains based rather than grass based, this has no external

7. How is it more convenient?  In any case, I feel we should have a 
function is_been_placed(x, y) rather then explicit check for the GRASS or 
PLAIN.

> I hope people like it.

We do.

On Tue, 7 Jan 2003, Guest via RT wrote:

> 
> This is the big one which re-does land placing in a way which I think is
> better, as described in the non-rt email.

Errr, actually more descriptive subjects would help, in the future.

G.