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

Give subgames the ability to disallow specific mapgens #6792

Merged
merged 5 commits into from Dec 16, 2017

Conversation

Ezhh
Copy link
Contributor

@Ezhh Ezhh commented Dec 15, 2017

This allows a subgame to control which mapgens show in the Mapgen dropdown when making a new game/world.

Mapgens which are not supported or otherwise suitable for a given subgame can be disallowed by adding a setting in game.conf. For example, if you wish to disable v6, flat and fractal mapgens, add:
disallowed_mapgens = v6,flat,fractal

This results in:
mapgensleft

Fixes #4768

@paramat paramat added @ Mainmenu @ Mapgen Feature ✨ PRs that add or enhance a feature labels Dec 15, 2017
@paramat
Copy link
Contributor

paramat commented Dec 15, 2017

Will test. I support the concept.

@Ezhh
Copy link
Contributor Author

Ezhh commented Dec 15, 2017

Thank you. Also please let me know if there is anywhere suitable to document this (other than wiki). It feels like it needs documentation, but none of the docs I see are suitable.

@@ -551,7 +551,7 @@ end
--------------------------------------------------------------------------------
if INIT == "mainmenu" then
function core.get_game(index)
local games = game.get_games()
local games = core.get_games()
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, was this a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup.

@paramat
Copy link
Contributor

paramat commented Dec 15, 2017

@Ezhh
Copy link
Contributor Author

Ezhh commented Dec 15, 2017

Added documentation.

@paramat
Copy link
Contributor

paramat commented Dec 15, 2017

Works well 👍

Copy link
Member

@rubenwardy rubenwardy left a comment

Choose a reason for hiding this comment

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

string.find may cause an issue if the mapgen name is contained in another mapgen name. Eg, if we add "v7 simple" or something in the future

local gamepath = minetest.get_game(gameidx).path
local gameconfig = Settings(gamepath.."/game.conf")

local disallowed_mapgens = gameconfig:get("disallowed_mapgens")
Copy link
Member

Choose a reason for hiding this comment

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

local disallowed_mapgens = gameconfig:get("disallowed_mapgens"):lower():split()
for key, value in pairs(disallowed_mapgens) do
     disallowed_mapgens[key] = value:trim()
end


if disallowed_mapgens then
for i = #mapgens, 1, -1 do
if string.find(disallowed_mapgens, mapgens[i]) then
Copy link
Member

Choose a reason for hiding this comment

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

if table.indexof(disallowed, mapgens[i]:lower()) > 0 then

@rubenwardy
Copy link
Member

Changing the subgame resets the text fields

@Ezhh
Copy link
Contributor Author

Ezhh commented Dec 16, 2017

Worldname fixed and no longer resets incorrcetly, thank you. Seed already saves correctly so shouldn't need any adjustment.

@rubenwardy
Copy link
Member

@paramat: still ok with this?

@ThomasMonroe314
Copy link
Contributor

This is a nice feature to have for modders to use.

disallowed_mapgens[key] = value:trim()
end

if disallowed_mapgens then
Copy link
Member

Choose a reason for hiding this comment

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

Superfluous check. string.split never returns nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rubenwardy requested it be this way. I know his commented version is a little different, but he asked me to adjust to this, so will see what he says.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't notice this aha, krock is right unfortunately. But doesn't matter too much

end
end

local gamepath = minetest.get_game(gameidx).path
Copy link
Member

Choose a reason for hiding this comment

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

core.get_game

@Ezhh
Copy link
Contributor Author

Ezhh commented Dec 16, 2017

Just to note: When you've had three approvals... and are still getting asked to change things on something that works and isn't controversial, there is something a bit unappealing with the development process.

@SmallJoker
Copy link
Member

@Ezhh Whereas the labels suggest the PR is not reviewed entirely yet - it apparently is. As you can see, the changes I requested are quite trivial and not required to make this PR work properly, they were requested for consistent and simple code.
Please note that reviews sometimes have to be refreshed when changes were done, hence, rubenwardy's question whether the approval is still standing. Nevertheless, it hasn't got any disapprovals which makes it ready to merge already now.

@Ezhh
Copy link
Contributor Author

Ezhh commented Dec 16, 2017

@SmallJoker I will be changing the other point you've raised, so hold on for that please. I simply don't want to change things that are already changes specifically asked for by developers without them weighing in again. I do understand how this works, but I also feel justified in being frustrated by it.

@SmallJoker SmallJoker merged commit 649eef9 into minetest:master Dec 16, 2017
@Xaleth
Copy link

Xaleth commented Dec 16, 2017

649eef9

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Dec 16, 2017

Wow, so this was coded, tested and merged and I only noticed it now? xD

Thanks, man! ❤️

@Ezhh
Copy link
Contributor Author

Ezhh commented Dec 16, 2017

@Wuzzy2 Very welcome. Hope it will be useful :)

@Megaf
Copy link
Contributor

Megaf commented Dec 17, 2017

🎉

@paramat
Copy link
Contributor

paramat commented Dec 17, 2017

Yes i'm still fine with it. The other devs are better with menu code than i am.

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

9 participants