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

Add mapgen settings to create world dialog #2561

Closed

Conversation

Projects
None yet
@srifqi
Copy link
Contributor

commented Mar 26, 2015

Any changes in mainmenu, change the minetest.conf.
Please run updatepo.sh after merge.

Require #2565 to run. (fixed)

  • Better name for "Light" (solved, thanks to @Rui914)
  • Found a bug, player cannot re-check checkbox after save (solved, require #2565)
  • Adapt to new mapgens and settings

minetest-201603051219 mainmenu mapgen v6 flags dialog

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2015

The old one (#2480) has deleted :(
So, I create another pr

minetest-201504142145 mainmenu mapgen flags dialog
^ If the snowbiomes enabled, jungle is force-enabled.

@Zeno-

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2015

This is starting to look good. The only problem is it doesn't seem to work for me :)

@srifqi srifqi changed the title Add mapgen specific flags to create world dialog [WIP] Add mapgen specific flags to create world dialog Mar 26, 2015

@C1ffisme

This comment has been minimized.

Copy link

commented Mar 26, 2015

Screenshots?

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2015

minetest-201503270609 mainmenu create world dialog flags
^ The Create World dialog (old)

minetest-201503270611 mainmenu mapgen flags dialog
^ The mapgen dialog (old)
^ That dialog shows depend on what mapgen did you choose

@C1ffisme

This comment has been minimized.

Copy link

commented Mar 27, 2015

👍

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2015

Updated,
Add mg_spflags param a.k.a. Generated structure
Require #2565 to merge
minetest-201503271346 mainmenu mapgen flags dialog

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2015

Updated,
Just know about core.is_yes. So, I removed #2565 (toboolean). Also, the code has changed.
Thanks, @est31!

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2015

@Rui914 Nice! Will update ASAP.

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2015

Updated, thanks to @Rui914.

All TODOs have completed. I'll squash when this accepted.
(Should I do it now?)

@srifqi srifqi changed the title [WIP] Add mapgen specific flags to create world dialog Add mapgen specific flags to create world dialog Mar 29, 2015

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2015

Quick update:

  • Always load default mapgen flags at start.
  • Change "Flags" to "Settings"
@rubenwardy

This comment has been minimized.

Copy link
Member

commented Mar 30, 2015

👍

@nerzhul

This comment has been minimized.

Copy link
Member

commented Mar 30, 2015

jenkins build please

@srifqi srifqi force-pushed the srifqi:add_mg_spflags_in_dlg_create_world branch from 45d3d67 to a5d9ec6 Mar 31, 2015

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2015

Okay, squashed.
Ready to merge. (see below)

@nerzhul

This comment has been minimized.

Copy link
Member

commented Mar 31, 2015

We are waiting the builds :)

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2015

Um... @Rui914 You commented twice :D

Also, why this pr does not have any labels?

@nerzhul

This comment has been minimized.

Copy link
Member

commented Mar 31, 2015

Because nobody set it ?

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2015

Okay, @nerzhul Thanks!

Also, all build has passed, successful checks.
Ready to merge. (see below)

@fz72

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2015

Why did you create a save button but no cancel button. Maybe you should add a save button and a cancel button. Only set the settings on save and do nothing on cancel. Maybe the dialog should look like the others with the save and cancel button at the bottom.

core.setting_set("mg_flags","trees, caves")
core.setting_set("mgv5_spflags","blobs")
core.setting_set("mgv6_spflags","biomeblend, jungles, mudflow")
core.setting_set("mgv7_spflags","mountains, ridges")

This comment has been minimized.

Copy link
@fz72

fz72 Apr 1, 2015

Contributor

Why not loading the default settings from minetest.conf?

This comment has been minimized.

Copy link
@srifqi

srifqi Apr 1, 2015

Author Contributor

We have to load the default (from: minetest.conf.example), so any player will create world without worrying about the parameter.

This comment has been minimized.

Copy link
@fz72

fz72 Apr 3, 2015

Contributor

We could load the default from minetest.conf. At beginning these are the defaults from minetest.conf.example. When you change the flags in the dialog you could store them in minetest.conf. Then the players have always their own default settings.

This comment has been minimized.

Copy link
@srifqi

srifqi Apr 5, 2015

Author Contributor

Nope. But you can make another pr when this get merged.
Okay, this also suggested by other.


return false
return ddhandled

This comment has been minimized.

Copy link
@fz72

fz72 Apr 1, 2015

Contributor

why you need ddhanled?

This comment has been minimized.

Copy link
@srifqi

srifqi Apr 1, 2015

Author Contributor

See at line 146-150. There is a dropdown that needs to be handled.

This comment has been minimized.

Copy link
@fz72

fz72 Apr 3, 2015

Contributor

I see it. But you return true in the dropdown. And only the dropdown is not handled you need to return false. I think you don't need ddhandled.

This comment has been minimized.

Copy link
@srifqi

srifqi Apr 5, 2015

Author Contributor

I leave it there for another things, if exist.

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2015

@fz72

Why did you create a save button but no cancel button. Maybe you should add a save button and a cancel button. Only set the settings on save and do nothing on cancel. Maybe the dialog should look like the others with the save and cancel button at the bottom.

I forgot that, because it always save anything every change.
Thanks,for reminding me!
It will be changed to "OK".

.

@ShadowNinja
Copy link
Member

left a comment

This sets the default values in minetest.conf instead of setting the actual world-specific values in the world configuration file.

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2017

Maybe we can bind to create world dialog instead of working independently? I'll try to figure out how.

@srifqi srifqi force-pushed the srifqi:add_mg_spflags_in_dlg_create_world branch from 4f1d7ba to d526c15 May 10, 2017

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2017

At this point, this will be only saved to minetest.conf after player click Create and will reset back to what minetest.conf has when player click Cancel. Maybe we should reset back to default instead of to what the content of minetest.conf is?

I see that we have Settings API, any suggestion for this?

srifqi added some commits Mar 26, 2015

Add mapgen settings to create world dialog
Any changes in mainmenu change the minetest.conf.
Supports new mapgens.
Only update minetest.conf when needed
Only write setting to minetest.conf when it is different than default or the setting already there.
Update flags lists.
Change from "Close" to "Save" and "Cancel".
Don't set for nil value by assuming uses default.
Use tostring instead of dump.
Only apply to minetest.conf when creating new world
Temporary flags and some function for loading and saving is now on global scope.
Change back to one button: Close.

@srifqi srifqi force-pushed the srifqi:add_mg_spflags_in_dlg_create_world branch from d526c15 to aa7ccbc May 23, 2017

@paramat

This comment has been minimized.

Copy link
Member

commented Jul 15, 2017

Currently i'm still very unsure about this, due to issues of behaviour. These settings are settable in .conf and advanced settings already so i'm not sure about a 3rd place.
It also makes working on mapgen more difficult, every new flag and parameter needs this code changing too.
So 👎 sorry, the intention is good. I think it's better we work on making advanced settings more user-friendly.

@C1ffisme

This comment has been minimized.

Copy link

commented Jul 15, 2017

@paramat This is usually where I would disagree for the sake of configuration, but to be fair, I don't think I really care about letting someone turn off biome blending, caves or trees. Some of this stuff only affects v6 now, and flat terrain is a totally different mapgen now.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2017

I would like to have possibility to configure those settings via gui during world creation.

@paramat

This comment has been minimized.

Copy link
Member

commented Jul 16, 2017

Thought on this more.
I know that the intention is good, and that some would like this, and i appreciate the amount of work that has gone into it, so i feel bad making this decision, but for several reasons i have to refuse this in any form, sorry.

  • World creation menu is for the most basic and essential settings only: name, seed, mapgen, and this is enough for most users. I deal with the mapgen questions on the forum and in IRC so i have a good idea of how many people need to alter flags, it's not as many as you would assume, and most are already able to using the existing methods.
  • Mapgen global and specific flags, and the non-noise parameters, are non-essential to change and are already settable in 2 ways in .conf and advanced settings, i want to avoid a 3rd way to set these to avoid duplication and non-essential extra code.
  • I feel we should instead work on advanced setting and organise the sections to be more intuitive, currently it is often guesswork, even for me, to know which initial section to expand.
  • Currently mapgen files are nicely self-contained. if i add or remove a parameter i don't have to touch any files other than mapgen file, conf.example or settingtypes.txt, and those 2 text files are simple and quick to update.
    With this PR i would have to completely reformat formspecs with every change, this will often be a bigger job than the mapgen work i am doing. This will be very irritating to have to do and i can tell you now i am not prepared to do that, editing just the 2 text files is already irritating and boring enough.
    Some may argue that 'others will maintain the extra code and make the menu changes you don't want to', this is not practical or realistic, it is and will be down to me to make complete PRs for mapgen and i need to be able to do so.
  • With our severe dev shortage we have to keep things simple if they possibly can be.

@paramat paramat added the Won't add label Jul 16, 2017

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

Any update on this?

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2017

IDK, maybe this won't be added? (From the tag set.)

@paramat

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

I am the MT mapgen dev so i know how this will affect dev, this will make my dev work much more difficult and greatly increase my workload, this is also not much needed.
Working on new mapgens as i do i add and remove mapgen flags very often, like daily, having to redesign a formspec each time will be unbearable.
So i'm sorry but there is absolutely no way i can allow this to go ahead, this will be hell for me. This type of non-essential complexity is the last thing we need with our severe lack of dev time.

Let's work on making advanced settings easier to use instead, that needs attention anyway.

The 'water level' setting (and 'ground level' in mgflat) is impractical, when you change that you also need to change up to 5 other 'level' settings (and cloud height) to make a world work, anyone capable of understanding what to adjust and capable of doing so will be capable enough to use existing settings to do this.

Most players, and the less capable ones, just want to play a mapgen with default options. Those who are capable enough to start customising mapgen are capable enough to use 'conf or advanced settings.

The intention is good and people will support due to the intention, but unfortunately it is not practical.

@paramat paramat closed this Aug 7, 2017

@paramat paramat removed the Possible close label Aug 8, 2017

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2017

So, that marks the end of this more than two years old PR. Thank you so much for your attentions.

I can understand how this will make map dev complicated. Also, I agree on making better Advanced Settings.

Happy coding, all! :D

@jastevenson303

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2017

I'm assuming the notion of mapgen flag combinations set up as a small number of selectable presets has already been brought up?

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2017

Let's move on to #3696 to make much better UI.

@paramat

This comment has been minimized.

Copy link
Member

commented Sep 14, 2017

New PR #6399

@paramat

This comment has been minimized.

Copy link
Member

commented Oct 21, 2017

I think we can possibly revive a small part of this to allow setting of a few basic global mapgen parameters during world creation, the advantage being these are set only for that world, which avoids having to change a setting back again afterwards.
These would probably be:

  1. mapgen_limit = 31000
  2. mg_flags = caves,dungeons,light,decorations but without light being settable as that is for advanced lua mapgen use.

Not water_level as changing this without re-registering a custom biome system makes a nasty mess that will give a bad impression. Mapgen Carpathian cannot cope with non-default water level.
Not chunksize as this needs understanding of the resullts before adjusting.

@srifqi

This comment has been minimized.

Copy link
Contributor Author

commented Oct 23, 2017

As #6399 already merged. Maybe we can start reviving?

@paramat

This comment has been minimized.

Copy link
Member

commented Oct 23, 2017

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.