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

Revival of Add minetest.generate_biomes and minetest.generate_biome_dust #15730

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kromka-chleba
Copy link
Contributor

@kromka-chleba kromka-chleba commented Jan 28, 2025

Attempted revival of #14260
Fixes #8329
Keep in mind that I have no C++ experience so this may be funny and could take some time.

Description of the PR:

As in the original: #14260

To do

This PR is READY

  • Rebase (surprisingly resolving the conflict was trivial)
  • Test and see that it still works
  • Test it with the mapgen env
  • Address reviewers' concerns from the original PR

I'm not sure about this #14260 (comment) but otherwise I hopefully addressed all review things.

EDIT: I think I fixed it here 05e4b7a

How to test

  1. Clone this mod to your minetest/mods/
  2. Enter the directory, switch to the use_core_biomegen branch
  3. Make a new MTG world
  4. Enable the lvm_example mod
  5. Witness it working
  6. Optionally switch to the master branch of lvm_example and see that it doesn't place biomes and dust.

Testing it in the mapgen env:

  1. Create a flatgen directory in your minetest/mods/
  2. Download the flatgen fork and put the files into flatgen
  3. Make a new MTG world
  4. Enable the flatgen mod
  5. Witness it working

@Zughy Zughy added @ Script API @ Mapgen Feature ✨ PRs that add or enhance a feature Concept approved Approved by a core dev: PRs welcomed! labels Jan 28, 2025
@kromka-chleba kromka-chleba force-pushed the lua_generate_biomes branch 3 times, most recently from 833583f to cf0f30a Compare January 28, 2025 21:45
If generate_ores/decorations is called after generate_biomes, the biome map
is taken into account.
@kromka-chleba kromka-chleba marked this pull request as ready for review January 28, 2025 21:55
@kromka-chleba
Copy link
Contributor Author

kromka-chleba commented Jan 28, 2025

Regarding this #14260 (comment)

Do you want me to load a bigger VM and read y -1? With the mapgen env the VM should already be big enough thanks to the overgeneration margin. What should I do with arbitrarily-sized VMs made by modders?

EDIT: I think I fixed it here 05e4b7a

I also adjusted the examples (removed the manual offset). If that's not the solution then I'm waiting for reviewers to show me the proper fix.

@kromka-chleba
Copy link
Contributor Author

@sfan5 would you be willing to continue reviewing this given that you were a reviewer for the original PR?
I hopefully addressed all your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature Help needed @ Mapgen @ Script API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose MapgenBasic::generateBiomes() to lua
3 participants