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

Allow multiple cave liquids in a biome definition #8481

Merged
merged 2 commits into from May 18, 2019
Merged

Allow multiple cave liquids in a biome definition #8481

merged 2 commits into from May 18, 2019

Conversation

paramat
Copy link
Contributor

@paramat paramat commented Apr 20, 2019

Although labelled a feature, this is an important step in de-hardcoding some aspects of mapgen.

We already have a biome-defined cave liquids feature, however only 1 cave liquid per biome can be defined, which cannot reproduce the current mix of water and lava caves.

This PR allows games to replace the hardcoded engine distribution of cave liquids with biome-defined cave liquids, while preserving the ability to have multiple cave liquids in a biome (such as water and lava).
Relative chances can be altered by repeated entries in the list.

If a biome defined cave liquid (or multiple) is defined, each entire cave uses that node (or a randomly chosen node from the list). Previously, biome-defined cave liquid was calculated per cave segment.

The change from per-segment to per-cave avoids every cave segment randomly choosing a node from the list, resulting in too-rapidly changing cave liquids within one cave. For example if the 2 nodes were water and lava, switching back and forth every few nodes would result in almost completely obsidian, no more lava lakes.

So the code for biome-defined liquid has moved from 'carveRoute()' (which generates each cave segment) to 'makeCave()' (which applies to the entire cave).

If no biome defined cave liquids are defined, the behaviour falls back to the old hardcoded behaviour.

There is a small performance benefit as biome is no longer being calculated for every cave segment.

This arose from discussion of MTG cave liquids:
minetest/minetest_game#2339 (comment)
minetest/minetest_game#2358

/////////////////////

Commit 2:
Don't place nodes if cave liquid is defined as 'air'

Avoids pointlessly placing huge numbers of "air" nodes when a cave liquid is defined to be air, for when no cave liquid is wanted.

@paramat paramat added @ Script API @ Mapgen WIP The PR is still being worked on by its author and not ready yet. Feature ✨ PRs that add or enhance a feature labels Apr 20, 2019
@paramat
Copy link
Contributor Author

paramat commented Apr 20, 2019

screenshot_20190420_030852

Works but having every cave segment a different liquid is bad.
EDIT: Fixed.

@paramat
Copy link
Contributor Author

paramat commented Apr 20, 2019

PR also does this:

If a biome defined cave liquid (or multiple) is defined, the entire cave uses that node (or a randomly chosen node from the list).
If multiple cave liquids are defined in a list, this avoids every cave segment randomly choosing a node from the list, resulting in too-rapidly changing cave liquids within one cave. For example if the 2 nodes were water and lava, switching back and forth every few nodes would result in almost completely obsidian, no more lava lakes.

If no biome defined cave liquids are defined, the behaviour falls back to the old hardcoded behaviour.

There is a small performance benefit as biome is no longer being calculated for every cave segment.

@paramat
Copy link
Contributor Author

paramat commented Apr 20, 2019

With:

	minetest.register_biome({
		name = "grassland_under",
		node_cave_liquid = {"default:meselamp", "default:mese_post_light"},
		y_max = -256,
		y_min = -31000,
		heat_point = 50,
		humidity_point = 35,
	})

In that biome:

screenshot_20190420_044614

screenshot_20190420_044605

Each entire cave uses one randomly chosen node from the list.

@paramat
Copy link
Contributor Author

paramat commented Apr 21, 2019

I'm seriously stuck.
In testing this seems to work but there is an error shown and i can't seem to silence it.
2019-04-20 02:02:22: ERROR[Main]: NodeResolver: no more node lists

@paramat
Copy link
Contributor Author

paramat commented Apr 21, 2019

Error seems fixed.
It was caused by the fallback 'default biome' that needed to 'push back' a list size of 1 for "node_cave_liquid". See line comment.

@@ -63,6 +63,7 @@ BiomeManager::BiomeManager(Server *server) :
b->m_nodenames.emplace_back("mapgen_stone");
b->m_nodenames.emplace_back("ignore");
b->m_nodenames.emplace_back("ignore");
b->m_nnlistsizes.push_back(1);
Copy link
Contributor Author

@paramat paramat Apr 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixed the error message.

@paramat
Copy link
Contributor Author

paramat commented Apr 22, 2019

Tested with a biome with all fields used, mod code (mod 'depends' on default and stairs):

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

minetest.register_biome({
	name = "test",
	node_dust = "default:snow",
	node_top = "default:dirt_with_snow",
	depth_top = 1,
	node_filler = "default:dirt",
	depth_filler = 3,
	node_stone = "default:sandstone",
	node_water_top = "default:ice",
	depth_water_top = 1,
	node_water = "default:permafrost",
	node_river_water = "default:lava_source",
	node_riverbed = "default:sand",
	depth_riverbed = 2,
	node_cave_liquid = {"default:meselamp", "default:mese_post_light"},
	node_dungeon = "default:stone_block",
	node_dungeon_alt = "default:sandstone_block",
	node_dungeon_stair = "stairs:stair_stone_block",
	--vertical_blend = ,
	y_max = 31000,
	y_min = -31000,
	heat_point = 50,
	humidity_point = 50,
})

When node_cave_liquid is left out, caves will revert to old hardcoded behaviour (water and lava).

@paramat paramat changed the title Allow a list cave liquids in a biome definition Allow multiple cave liquids in a biome definition Apr 22, 2019
@paramat paramat removed the WIP The PR is still being worked on by its author and not ready yet. label Apr 22, 2019
@SmallJoker
Copy link
Member

This is a nice feature, but I'd like to test its functionality as soon I've got time for it.
Just to mention that this PR isn't forgotten.

@paramat
Copy link
Contributor Author

paramat commented May 14, 2019

Simple commit 2 added, see first post.

SmallJoker, dev is unbearably slow at the moment so i suggest not testing to speed up approval, you can trust my testing. I'm getting impatient and feel i should be partially trusted with mapgen stuff =)

This allows games to specify biome cave liquids and avoid the old
hardcoded behaviour, but preserves the ability to have multiple
cave liquids in one biome, such as lava and water.

When multiple cave liquids are defined by the biome definition,
make each entire cave use a randomly chosen liquid, instead of
every small cave segment using a randomly chosen liquid.
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected.

@paramat paramat merged commit b1b40fe into minetest:master May 18, 2019
@paramat paramat deleted the multicaveliq branch July 14, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants