Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase maximum number of biomes above 255 #9694

Closed
paramat opened this issue Apr 16, 2020 · 13 comments · Fixed by #9855
Closed

Increase maximum number of biomes above 255 #9694

paramat opened this issue Apr 16, 2020 · 13 comments · Fixed by #9855
Assignees
Labels
@ Mapgen Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature

Comments

@paramat
Copy link
Contributor

paramat commented Apr 16, 2020

This is something i would like to do, opening an issue in case there is useful input or any objections.
https://github.com/minetest/minetest/blob/7539267d370ae9a1b547008a937bd7f57bece541/src/mapgen/mg_biome.h#L35
I suggest u8 -> u16.

During discussions on the forum, i have become aware of some users who are approaching the limit of 255 biomes. Also, i have become aware of, and sometimes think of, certain useful biome system design techniques that require large numbers of biomes (for example, multiple identical biomes each with slightly differing heat/humidity points, in order to create a desired distribution).

Some biome system designers are approaching roughly 32 surface biomes, which is reasonable. Now consider that each heat/humidty point could possibly have a vertical stack of underground and high altitude biomes, a stack of 8 is reasonable. 32 * 8 = 256. So it is entirely reasonable to allow over 255 biomes, and this creates some interesting new possibiliites.

@paramat paramat added @ Mapgen Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature labels Apr 16, 2020
@paramat paramat self-assigned this Apr 16, 2020
@sfan5
Copy link
Collaborator

sfan5 commented Apr 17, 2020

Unrelated, but before any underlying changes are made to mapgen(s) it'd be good if #9627 could be merged first.

@nerzhul
Copy link
Contributor

nerzhul commented Apr 17, 2020

i agree with sfan5, and we should ensure that we don't serialize the biome id somewhere as u8

@paramat
Copy link
Contributor Author

paramat commented Apr 17, 2020

sfan5, i agree, this can wait.
However, if we are considering mapgen PR priority, then #9254 and #9296 are highest priority.
#9296 should have been in 5.2.0 =) Luckily it probably has no conflict with #9627

@paramat
Copy link
Contributor Author

paramat commented May 8, 2020

#9627 is merged, will start work on this.

@nerzhul
Copy link
Contributor

nerzhul commented May 8, 2020

just a question, in which case we can have more than dozens of biimes, that sounds a little bit crazy, no ?

@paramat
Copy link
Contributor Author

paramat commented May 9, 2020

MTG has 43 biomes already.

Remember that each heat/humidity point has multiple vertically-stacked biomes.
One heat/humidty point can have: several layers of underground biomes, seabed biome, shallow shore biome, beach biome, sand dune biome, lowland biome, highland biome, plus another cycle of multiple biomes for the floatland layer in mgv7.
So there could easily be a vertical stack of 16 per heat/humidity point.
Now consider a not remotely excessive 16 heat/humidity points, 16 * 16 = 256.

This was explained in the first post.

u8 was chosen many years ago when mgv6 was still dominating people's thinking and people were not really thinking about vertically stacking biomes.

@paramat
Copy link
Contributor Author

paramat commented May 10, 2020

Hmm something weird ...

I wrote a mod to test that > 255 biomes works, for when i make a PR:

  • This registers 8 nodes with differing colours.
  • This then registers all the biomes, each with a different name, defining 'top node' as one of the 8 colours, such that the total number of biomes is divided equally into the 8 colours. This is done so that a world can be mapped and it can be checked that each colour is present and has very roughly equal area.
    The heat / humidity points are equally spaced in a lattice.
  • Finally the list of registered biome names (from minetest.registered_biomes) is printed out and the total number added up and printed.

I ran this mod in recent master with 511 biomes registered to see how master breaks in this situation.
But results suggest all 511 biomes were actually registered and are functional.
Can > 255 biomes already be used or have i made a mistake?
However, we should still change that u8 to a u16 if > 255 is expected to happen.

Resulting map has all 8 colours in very roughly equal proportions:

biomes255e512

Resulting printed list apparently has 511 entries:

biome_230
biome_238
biome_504
biome_150
biome_510
biome_509
biome_46
..
..
..
biome_380
biome_1
biome_276
biome_54
biome_464
biome_411
biome_20
biome_111
biome_124
511

Mod code (use with MTG, mod depends on 'default'):

for n = 1, 8 do
	minetest.register_node("biomeu16:" .. n, {
		description = "Colour " .. n,
		tiles = {"biomeu16_" .. n .. ".png"},
		groups = {cracky = 3},
	})
end

minetest.clear_registered_biomes()
minetest.clear_registered_ores()
minetest.clear_registered_decorations()

local nb = 512
local nbr = math.floor(math.sqrt(nb))
local pf = 80 / nbr
for b = 1, nb - 1 do
	minetest.register_biome({
		name = "biome_" .. b,
		node_top = "biomeu16:" .. (math.ceil(b / (nb / 8))),
		depth_top = 1,
		y_max = 31000,
		y_min = -31000,
		heat_point = math.floor(b / nbr) * pf + 10,
		humidity_point = math.fmod(b, nbr) * pf + 10,
	})
end

local nrb = 0
for k, v in pairs(minetest.registered_biomes) do
	print(k)
	nrb = nrb + 1
end
print(nrb)

@paramat
Copy link
Contributor Author

paramat commented May 11, 2020

I get a feeling my way of testing is deceiving. I get a feeling that on mod load, the number of registered biomes is unlimited, but when the environment starts the number of biome IDs may be limited to 255. Will test further.

@paramat
Copy link
Contributor Author

paramat commented May 11, 2020

Added another test to my code:

local function print_biome_ids()
	for k, v in pairs(minetest.registered_biomes) do
		print(minetest.get_biome_id(k))
	end
end

minetest.after(5, print_biome_ids)

Getting the biome ID from name only works after environment startup.
The resulting list is a list of biome IDs, the weird number order matches the previous list, and there are also 511 entries.
Still not convinced there are actually 511 functional biomes in the world, i feel i need to test that more carefully.

@paramat
Copy link
Contributor Author

paramat commented May 11, 2020

Ugh, more testing. Still suggests all 511 biomes are functional in a world , but i cannot see how.
The difficult part of this issue is working out a way to check that > 255 biomes are present in a world. I am not confident about my method of testing this.

@paramat
Copy link
Contributor Author

paramat commented May 11, 2020

Tested in a more reliable way in master, again suggests > 255 biomes can be functional in a world. Still wondering if my testing is misleading me.

@paramat
Copy link
Contributor Author

paramat commented May 12, 2020

Interesting discussion from past IRC that might be the answer:
http://irc.minetest.net/minetest-dev/2018-06-21#i_5339139

14:52 Krock biomes have an internal index due to their inheritance, but we've also got biome_t which is u8, whereas the index is u32.
14:52 Krock So why are those different? How does biome_t differ from ->index at all?

@paramat
Copy link
Contributor Author

paramat commented May 12, 2020

Ah ... if you do a search for 'biome_t', that is only used for the value type of the mapgen object biomemap. This suggests that the only limitation to 255 occurs in the biomemap, which i have not tested yet. Will test, and this suggests a way to test the PR too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Mapgen Request / Suggestion The issue makes a suggestion for something that should be done but isn't a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants