Nice ! C++11 constructor :D

if i remember this enum should be ordered by mapgen addition instead of names, @paramat can you confirm ?

It prevents mapgen ID mismatch

This enum was created in this form in hmmmm's mapgen reorganisation commit 9270530
So i'm sure it's fine as it is.
However any new mapgens should perhaps be added after 'invalid', not sure if that is essential but seems the safest thing to do with an enum.

I changed the enum to this order:
enum MapgenType { MAPGEN_V5, MAPGEN_V6, MAPGEN_V7, MAPGEN_FLAT, MAPGEN_FRACTAL, MAPGEN_VALLEYS, MAPGEN_SINGLENODE, MAPGEN_INVALID, MAPGEN_CARPATHIAN, };

but it compiles with error:

/Users/vlapsley/games/minetest/src/mapgen.cpp:93:1: error: 'registered_mapgens_is_wrong_size'
declared as an array with a negative size
STATIC_ASSERT(
^~~~~~~~~~~~~~
/Users/vlapsley/games/minetest/src/util/basic_macros.h:50:36: note: expanded from macro
'STATIC_ASSERT'
UNUSED_ATTRIBUTE typedef char msg[!!(expr) * 2 - 1]
^~~~~~~~~~~~~~~~
1 error generated.

STATIC_ASSERT(
	ARRLEN(g_reg_mapgens) == MAPGEN_INVALID,
registered_mapgens_is_wrong_size);

Looks like 'invalid' needs to be the last one, so add it just before 'invalid'?

yeah invalid should be last

Since you are using large pseudorandom caves 2 more parameters are needed to be able to shift those caves and the lava they contain vertically if terrain is shifted vertically, see mgv7:

	this->large_cave_depth    = params->large_cave_depth;
	this->lava_depth          = params->lava_depth;

You can leave out lava_depth if you want as it has a default of y = -256, but only if terrain is locked to y = 1.

mnt_var noise is:

+	np_mnt_var       = NoiseParams(0,  1,  v3f(499,  499,  499),  2490,  5, 0.6,  2.0);

in case you did not know, with scale 1 and 5 octaves persistence 0.6 the value will actually vary between roughly -2 and 2.
Specifically the variation will be 1 + 0.6 + 0.6^2 + 0.6^3 + 0.6^4.
So if you want you might prefer to limit mod to -1 to 1.

However, when such limits are hit they can create 'edges' visible in the terrain. If you are happy with the current terrain no need to change it.

I'm happy with the way it is. As you say, clamping the upper and lower limit will create edges.

I played with the noise, reducing scale and octaves separately, but I found some of my favourite features where the noise is amplified by two ranges meeting together lost its charm.

If a player wants to reduce the occasional craziness of some mountains, this is the noise parameter to change.

You could speed up this function a bit:

1. Inline
2. One multiplication less: return noise1 - mod * (noise1 + noise2)
   We already have a similar, simplified function in the noise code but that one would be for (1 - mod)

Not essential but i was advised to use MTs builtin functions MYMIN() and MYMAX().

I've got no problem changing to MYMIN and MYMAX functions.

I was going to ask why use these functions over the cmath library, but I've just read that fmin/fmax were introduced in C++11 and may not work with older compilers.

So I will make the changes.

we are using c++11 now so it's fine, leave it as it is.

Ok, i was advised by hmmmm to use MYMIN/MYMAX but this was a while ago.

yeah its was at the era of non portable C++

Now that we use C++11, std::fmax is probably the better choice - more a standard than our Minetest-specific MYMAX function.

Yes i will start to use std::fmain/std::fmax from now on.

Seems here you are using y = 1 (sea level) as the grad zero point and compressing the terrain variation below sea level.
Sea level is actually variable using the global mapgen parameter water_level so:

+				int grad = (water_level - y);
+				if (y < water_level) {
+					grad *= 3;
+				}

This must be why it was reported that changing water_level messes stuff up.

I'll tell you one of my mapgen tricks.
If you add a vertical_scale parameter, default 1, and do:

+				int grad = (water_level - y) / vertical_scale;

The parameter acts like a master control on terrain vertical scale, easily changed without editing many confusing noise parameters.

The main use for this is the following:
Set vertical_scale to 0.1.
Divide all noise parameter 'spread' values (v3f(1889, 1889, 1889)) by 10.
This shrinks the terrain by a factor of 10 both vertically and horizontally, creating a 1/10th scale 3D map of your mapgen. To create a map disable biomes, caves and decorations (clear the registered biomes in a mod).

My initial work did use a vertical_scale parameter. But I had awful problems with worlds sometimes not spawning stone - only air and water.

Or, sometimes you would spawn in the ground and the ground (completely flat) would be at y=180 and mountains would be going up to y=400 or 500 and canyons down to water level. It was beautiful and wrong at the same time.

Perhaps I had something else incorrect in the code that caused this though.

Don't ask how many times I rewrote this mapgen from scratch. I'm still coding with learner-plates on.

No problem, i'm not asking for this to be included.

s32 instead of int
grad_wl can be a private class member calculated once at line 63 in the .cpp constructor

s32 instead of int
no brackets

since action is a single line no need for curly brackets

s32 instead of int
grad_wl can be a private class member calculated once at line 63 in the .cpp constructor

s32 instead of int
no brackets

since action is a single line no need for curly brackets

add s32 grad_wl;

Could use a ternary operator to reduce all this to a single line:

s32 grad = (y < water_level) ? grad_wl + (water_level - y) * 3 : 1 - y;

Could use a ternary operator to reduce all this to a single line:

s32 grad = (y < water_level) ? grad_wl + (water_level - y) * 3 : 1 - y;

No need for s32 as the variable has been initialised as s32 in the .h file.

Ah, thanks. Now I understand about the unused variable warning. Fixed.