Improve Lua settings menu

* Add key settings to setting table and ignore them later
  This way they are added to the auto-generated minetest.conf.example
* Add flags type
* Add input validation for int, float and flags
* Break in-game graphic settings into multiple sections
* Parse settingtpes.txt in mods and games
* Improve description for a lot of settings
* Fix typos and wording in settingtypes.txt
* Convert language setting to an enum

builtin/mainmenu/tab_settings.lua

@@ -17,34 +17,36 @@
 
 local FILENAME = "settingtypes.txt"
 
-local function parse_setting_line(settings, line)
+local CHAR_CLASSES = {
+	SPACE = "[%s]",
+	VARIABLE = "[%w_%-%.]",
+	INTEGER = "[-]?[%d]",
+	FLOAT = "[-]?[%d%.]",
+	FLAGS = "[%w_%-%.,]",
+}
+
+-- returns error message, or nil
+local function parse_setting_line(settings, line, read_all, base_level)
 	-- empty lines
-	if line:match("^[%s]*$") then
+	if line:match("^" .. CHAR_CLASSES.SPACE .. "*$") then
 		-- clear current_comment so only comments directly above a setting are bound to it
 		settings.current_comment = ""
 		return
 	end
 
 	-- category
-	local category = line:match("^%[([^%]]+)%]$")
+	local stars, category = line:match("^%[([%*]*)([^%]]+)%]$")
 	if category then
-		local level = 0
-		local index = 1
-		while category:sub(index, index) == "*" do
-			level = level + 1
-			index = index + 1
-		end
-		category = category:sub(index, -1)
 		table.insert(settings, {
 			name = category,
-			level = level,
+			level = stars:len() + base_level,
 			type = "category",
 		})
 		return
 	end
 
 	-- comment
-	local comment = line:match("^#[%s]*(.*)$")
+	local comment = line:match("^#" .. CHAR_CLASSES.SPACE .. "*(.*)$")
 	if comment then
 		if settings.current_comment == "" then
 			settings.current_comment = comment
@@ -54,9 +56,20 @@ local function parse_setting_line(settings, line)
 		return
 	end
 
+	local error_msg
+
 	-- settings
-	local first_part, name, readable_name, setting_type =
+	local first_part, name, readable_name, setting_type = line:match("^"
-			line:match("^(([%w%._-]+)[%s]+%(([^%)]*)%)[%s]+([%w_]+)[%s]*)")
+			-- this first capture group matches the whole first part,
+			--  so we can later strip it from the rest of the line
+			.. "("
+				.. "([" .. CHAR_CLASSES.VARIABLE .. "+)" -- variable name
+				.. CHAR_CLASSES.SPACE
+				.. "%(([^%)]*)%)"  -- readable name
+				.. CHAR_CLASSES.SPACE
+				.. "(" .. CHAR_CLASSES.VARIABLE .. "+)" -- type
+				.. CHAR_CLASSES.SPACE .. "?"
+			.. ")")
 
 	if first_part then
 		if readable_name == "" then
@@ -65,14 +78,15 @@ local function parse_setting_line(settings, line)
 		local remaining_line = line:sub(first_part:len() + 1)
 
 		if setting_type == "int" then
-			local default, min, max = remaining_line:match("^([%d]+)[%s]*([%d]*)[%s]*([%d]*)$")
+			local default, min, max = remaining_line:match("^"
+					-- first int is required, the last 2 are optional
+					.. "(" .. CHAR_CLASSES.INTEGER .. "+)" .. CHAR_CLASSES.SPACE .. "?"
+					.. "(" .. CHAR_CLASSES.INTEGER .. "*)" .. CHAR_CLASSES.SPACE .. "?"
+					.. "(" .. CHAR_CLASSES.INTEGER .. "*)"
+					.. "$")
 			if default and tonumber(default) then
-				if min == "" then
+				min = tonumber(min)
-					min = nil
+				max = tonumber(max)
-				end
-				if max == "" then
-					max = nil
-				end
 				table.insert(settings, {
 					name = name,
 					readable_name = readable_name,
@@ -83,12 +97,14 @@ local function parse_setting_line(settings, line)
 					comment = settings.current_comment,
 				})
 			else
-				core.log("error", "Found invalid int in " .. FILENAME .. ": " .. line)
+				error_msg = "Invalid integer setting"
 			end
 
-		elseif setting_type == "string" or setting_type == "flags" or setting_type == "noise_params" then
+		elseif setting_type == "string" or setting_type == "noise_params"
-			local default = remaining_line:match("^[%s]*(.*)$")
+				or setting_type == "key" then
+			local default = remaining_line:match("^(.*)$")
 			if default then
+				if setting_type ~= "key" or read_all then -- ignore key type if read_all is false
 					table.insert(settings, {
 						name = name,
 						readable_name = readable_name,
@@ -96,8 +112,9 @@ local function parse_setting_line(settings, line)
 						default = default,
 						comment = settings.current_comment,
 					})
+				end
 			else
-				core.log("error", "Found invalid string in " .. FILENAME .. ": " .. line)
+				error_msg =  "Invalid string setting"
 			end
 
 		elseif setting_type == "bool" then
@@ -110,19 +127,19 @@ local function parse_setting_line(settings, line)
 					comment = settings.current_comment,
 				})
 			else
-				core.log("error", "Found invalid bool in " .. FILENAME .. ": " .. line)
+				error_msg =  "Invalid boolean setting"
 			end
 
 		elseif setting_type == "float" then
-			local default, min, max
+			local default, min, max = remaining_line:match("^"
-					= remaining_line:match("^([%d%.]+)[%s]*([%d%.]*)[%s]*([%d%.]*)$")
+					-- first float is required, the last 2 are optional
+					.. "(" .. CHAR_CLASSES.FLOAT .. "+)" .. CHAR_CLASSES.SPACE .. "?"
+					.. "(" .. CHAR_CLASSES.FLOAT .. "*)" .. CHAR_CLASSES.SPACE .. "?"
+					.. "(" .. CHAR_CLASSES.FLOAT .. "*)"
+					.."$")
 			if default and tonumber(default) then
-				if min == "" then
+				min = tonumber(min)
-					min = nil
+				max = tonumber(max)
-				end
-				if max == "" then
-					max = nil
-				end
 				table.insert(settings, {
 					name = name,
 					readable_name = readable_name,
@@ -133,26 +150,26 @@ local function parse_setting_line(settings, line)
 					comment = settings.current_comment,
 				})
 			else
-				core.log("error", "Found invalid float in " .. FILENAME .. ": " .. line)
+				error_msg =  "Invalid float setting"
 			end
 
 		elseif setting_type == "enum" then
-			local default, values = remaining_line:match("^([^%s]+)[%s]+(.+)$")
+			local default, values = remaining_line:match("^(.+)" .. CHAR_CLASSES.SPACE .. "(.+)$")
 			if default and values ~= "" then
 				table.insert(settings, {
 					name = name,
 					readable_name = readable_name,
 					type = "enum",
 					default = default,
-					values = values:split(","),
+					values = values:split(",", true),
 					comment = settings.current_comment,
 				})
 			else
-				core.log("error", "Found invalid enum in " .. FILENAME .. ": " .. line)
+				error_msg =  "Invalid enum setting"
 			end
 
 		elseif setting_type == "path" then
-			local default = remaining_line:match("^[%s]*(.*)$")
+			local default = remaining_line:match("^(.*)$")
 			if default then
 				table.insert(settings, {
 					name = name,
@@ -162,50 +179,143 @@ local function parse_setting_line(settings, line)
 					comment = settings.current_comment,
 				})
 			else
-				core.log("error", "Found invalid path in " .. FILENAME .. ": " .. line)
+				error_msg =  "Invalid path setting"
 			end
 
-		elseif setting_type == "key" then
+		elseif setting_type == "flags" then
-			--ignore keys, since we have a special dialog for them
+			local default, possible = remaining_line:match("^"
+					.. "(" .. CHAR_CLASSES.FLAGS .. "+)" .. CHAR_CLASSES.SPACE .. ""
+					.. "(" .. CHAR_CLASSES.FLAGS .. "+)"
+					.. "$")
+			if default and possible then
+				table.insert(settings, {
+					name = name,
+					readable_name = readable_name,
+					type = "flags",
+					default = default,
+					possible = possible,
+					comment = settings.current_comment,
+				})
+			else
+				error_msg =  "Invalid flags setting"
+			end
 
 		-- TODO: flags, noise_params (, struct)
 
 		else
-			core.log("error", "Found setting with invalid setting type in " .. FILENAME .. ": " .. line)
+			error_msg =  "Invalid setting type \"" .. type .. "\""
 		end
 	else
-		core.log("error", "Found invalid line in " .. FILENAME .. ": " .. line)
+		error_msg =  "Invalid line"
 	end
 	-- clear current_comment since we just used it
 	-- if we not just used it, then clear it since we only want comments
 	--  directly above the setting to be bound to it
 	settings.current_comment = ""
+
+	return error_msg
 end
 
-local function parse_config_file()
+local function parse_single_file(file, filepath, read_all, result, base_level)
-	local file = io.open(core.get_builtin_path() .. DIR_DELIM .. FILENAME, "r")
+	-- store this helper variable in the table so it's easier to pass to parse_setting_line()
+	result.current_comment = ""
+
+	local line = file:read("*line")
+	while line do
+		local error_msg = parse_setting_line(result, line, read_all, base_level)
+		if error_msg then
+			core.log("error", error_msg .. " in " .. filepath .. " \"" .. line .. "\"")
+		end
+		line = file:read("*line")
+	end
+
+	result.current_comment = nil
+end
+
+-- read_all: whether to ignore certain setting types for GUI or not
+-- parse_mods: whether to parse settingtypes.txt in mods and games
+local function parse_config_file(read_all, parse_mods)
+	local builtin_path = core.get_builtin_path() .. DIR_DELIM .. FILENAME
+	local file = io.open(builtin_path, "r")
 	local settings = {}
 	if not file then
 		core.log("error", "Can't load " .. FILENAME)
 		return settings
 	end
 
-	-- store this helper variable in the table so it's easier to pass to parse_setting_line()
+	parse_single_file(file, builtin_path, read_all, settings, 0)
-	settings.current_comment = ""
-
-	local line = file:read("*line")
-	while line do
-		parse_setting_line(settings, line)
-		line = file:read("*line")
-	end
-
-	settings.current_comment = nil
 
 	file:close()
+
+	if parse_mods then
+		-- Parse games
+		local games_category_initialized = false
+		local index = 1
+		local game = gamemgr.get_game(index)
+		while game do
+			local path = game.path .. DIR_DELIM .. FILENAME
+			local file = io.open(path, "r")
+			if file then
+				if not games_category_initialized then
+					local translation = fgettext_ne("Games"), -- not used, but needed for xgettext
+					table.insert(settings, {
+						name = "Games",
+						level = 0,
+						type = "category",
+					})
+					games_category_initialized = true
+				end
+
+				table.insert(settings, {
+					name = game.name,
+					level = 1,
+					type = "category",
+				})
+
+				parse_single_file(file, path, read_all, settings, 2)
+
+				file:close()
+			end
+
+			index = index + 1
+			game = gamemgr.get_game(index)
+		end
+
+		-- Parse mods
+		local mods_category_initialized = false
+		local mods = {}
+		get_mods(core.get_modpath(), mods)
+		for _, mod in ipairs(mods) do
+			local path = mod.path .. DIR_DELIM .. FILENAME
+			local file = io.open(path, "r")
+			if file then
+				if not mods_category_initialized then
+					local translation = fgettext_ne("Mods"), -- not used, but needed for xgettext
+					table.insert(settings, {
+						name = "Mods",
+						level = 0,
+						type = "category",
+					})
+					mods_category_initialized = true
+				end
+
+				table.insert(settings, {
+					name = mod.name,
+					level = 1,
+					type = "category",
+				})
+
+				parse_single_file(file, path, read_all, settings, 2)
+
+				file:close()
+			end
+		end
+	end
+
 	return settings
 end
 
-local settings = parse_config_file()
+local settings = parse_config_file(false, true)
 local selected_setting = 1
 
 local function get_current_value(setting)
@@ -236,16 +346,28 @@ local function create_change_setting_formspec(dialogdata)
 
 	local comment_text = ""
 
-	-- fgettext_ne("") doesn't have to return "", according to specification of gettext(3)
 	if setting.comment == "" then
 		comment_text = fgettext_ne("(No description of setting given)")
 	else
 		comment_text = fgettext_ne(setting.comment)
 	end
-	for _, comment_line in ipairs(comment_text:split("\n")) do
+	for _, comment_line in ipairs(comment_text:split("\n", true)) do
 		formspec = formspec .. "," .. core.formspec_escape(comment_line) .. ","
 	end
 
+	if setting.type == "flags" then
+		formspec = formspec .. ",,"
+				.. "," .. fgettext("Please enter a comma seperated list of flags.") .. ","
+				.. "," .. fgettext("Possible values are: ")
+				.. core.formspec_escape(setting.possible:gsub(",", ", ")) .. ","
+	elseif setting.type == "noise_params" then
+		formspec = formspec .. ",,"
+				.. "," .. fgettext("Format: <offset>, <scale>, (<spreadX>, <spreadY>, <spreadZ>), <seed>, <octaves>, <persistence>") .. ","
+				.. "," .. fgettext("Optionally the lacunarity can be appended with a leading comma.") .. ","
+	end
+
+	formspec = formspec:sub(1, -2) -- remove trailing comma
+
 	formspec = formspec .. ";1]"
 
 	if setting.type == "bool" then
@@ -256,7 +378,8 @@ local function create_change_setting_formspec(dialogdata)
 			selected_index = 1
 		end
 		formspec = formspec .. "dropdown[0.5,3.5;3,1;dd_setting_value;"
-				.. fgettext("Disabled") .. "," .. fgettext("Enabled") .. ";" .. selected_index .. "]"
+				.. fgettext("Disabled") .. "," .. fgettext("Enabled") .. ";"
+				.. selected_index .. "]"
 
 	elseif setting.type == "enum" then
 		local selected_index = 0
@@ -285,8 +408,20 @@ local function create_change_setting_formspec(dialogdata)
 
 	else
 		-- TODO: fancy input for float, int, flags, noise_params
-		formspec = formspec .. "field[0.5,4;9.5,1;te_setting_value;;"
+		local width = 10
-				.. core.formspec_escape(get_current_value(setting)) .. "]"
+		local text = get_current_value(setting)
+		if dialogdata.error_message then
+			formspec = formspec .. "tablecolumns[color;text]" ..
+			"tableoptions[background=#00000000;highlight=#00000000;border=false]" ..
+			"table[5,4;5,1;error_message;#FF0000,"
+					.. core.formspec_escape(dialogdata.error_message) .. ";0]"
+			width = 5
+			if dialogdata.entered_text then
+				text = dialogdata.entered_text
+			end
+		end
+		formspec = formspec .. "field[0.5,4;" .. width .. ",1;te_setting_value;;"
+				.. core.formspec_escape(text) .. "]"
 	end
 	return formspec
 end
@@ -303,6 +438,52 @@ local function handle_change_setting_buttons(this, fields)
 			local new_value = fields["dd_setting_value"]
 			core.setting_set(setting.name, new_value)
 
+		elseif setting.type == "int" then
+			local new_value = tonumber(fields["te_setting_value"])
+			if not new_value or math.floor(new_value) ~= new_value then
+				this.data.error_message = fgettext_ne("Please enter a valid integer.")
+				this.data.entered_text = fields["te_setting_value"]
+				core.update_formspec(this:get_formspec())
+				return true
+			end
+			if setting.min and new_value < setting.min then
+				this.data.error_message = fgettext_ne("The value must be greater than $1.", setting.min)
+				this.data.entered_text = fields["te_setting_value"]
+				core.update_formspec(this:get_formspec())
+				return true
+			end
+			if setting.max and new_value > setting.max then
+				this.data.error_message = fgettext_ne("The value must be lower than $1.", setting.max)
+				this.data.entered_text = fields["te_setting_value"]
+				core.update_formspec(this:get_formspec())
+				return true
+			end
+			core.setting_set(setting.name, new_value)
+
+		elseif setting.type == "float" then
+			local new_value = tonumber(fields["te_setting_value"])
+			if not new_value then
+				this.data.error_message = fgettext_ne("Please enter a valid number.")
+				this.data.entered_text = fields["te_setting_value"]
+				core.update_formspec(this:get_formspec())
+				return true
+			end
+			core.setting_set(setting.name, new_value)
+
+		elseif setting.type == "flags" then
+			local new_value = fields["te_setting_value"]
+			for _,value in ipairs(new_value:split(",", true)) do
+				value = value:trim()
+				if not value:match(CHAR_CLASSES.FLAGS .. "+")
+						or not setting.possible:match("[,]?" .. value .. "[,]?") then
+					this.data.error_message = fgettext_ne("\"" .. value .. "\" is not a valid flag.")
+					this.data.entered_text = fields["te_setting_value"]
+					core.update_formspec(this:get_formspec())
+					return true
+				end
+			end
+			core.setting_set(setting.name, new_value)
+
 		else
 			local new_value = fields["te_setting_value"]
 			core.setting_set(setting.name, new_value)
@@ -345,7 +526,8 @@ local function create_settings_formspec(tabview, name, tabdata)
 
 		if entry.type == "category" then
 			current_level = entry.level
-			formspec = formspec .. "#FFFF00," .. current_level .. "," .. core.formspec_escape(name) .. ",,"
+			formspec = formspec .. "#FFFF00," .. current_level .. "," .. fgettext(name) .. ",,"
+
 		elseif entry.type == "bool" then
 			local value = get_current_value(entry)
 			if core.is_yes(value) then
@@ -355,6 +537,10 @@ local function create_settings_formspec(tabview, name, tabdata)
 			end
 			formspec = formspec .. "," .. (current_level + 1) .. "," .. core.formspec_escape(name) .. ","
 					.. value .. ","
+
+		elseif entry.type == "key" then
+			-- ignore key settings, since we have a special dialog for them
+
 		else
 			formspec = formspec .. "," .. (current_level + 1) .. "," .. core.formspec_escape(name) .. ","
 					.. core.formspec_escape(get_current_value(entry)) .. ","
@@ -431,6 +617,13 @@ local function handle_settings_buttons(this, fields, tabname, tabdata)
 	return false
 end
 
+tab_settings = {
+	name = "settings",
+	caption = fgettext("Settings"),
+	cbf_formspec = create_settings_formspec,
+	cbf_button_handler = handle_settings_buttons,
+}
+
 local function create_minetest_conf_example()
 	local result = "#    This file contains a list of all available settings and their default value for minetest.conf\n" ..
 			"\n" ..
@@ -447,6 +640,7 @@ local function create_minetest_conf_example()
 			"#    http://wiki.minetest.net/\n" ..
 			"\n"
 
+	local settings = parse_config_file(true, false)
 	for _, entry in ipairs(settings) do
 		if entry.type == "category" then
 			if entry.level == 0 then
@@ -458,8 +652,8 @@ local function create_minetest_conf_example()
 				result = result .. "# " .. entry.name .. "\n\n"
 			end
 		else
-			if entry.comment_line ~= "" then
+			if entry.comment ~= "" then
-				for _, comment_line in ipairs(entry.comment:split("\n")) do
+				for _, comment_line in ipairs(entry.comment:split("\n", true)) do
 					result = result .."#    " .. comment_line .. "\n"
 				end
 			end
@@ -473,6 +667,9 @@ local function create_minetest_conf_example()
 			if entry.values then
 				result = result .. " values: " .. table.concat(entry.values, ", ")
 			end
+			if entry.possible then
+				result = result .. " possible values: " .. entry.possible:gsub(",", ", ")
+			end
 			result = result .. "\n"
 			result = result .. "# " .. entry.name .. " = ".. entry.default .. "\n\n"
 		end
@@ -485,6 +682,8 @@ local function create_translation_file()
 			"// It conatins a bunch of fake gettext calls, to tell xgettext about the strings in config files\n" ..
 			"// To update it, refer to the bottom of builtin/mainmenu/tab_settings.lua\n\n" ..
 			"fake_function() {\n"
+
+	local settings = parse_config_file(true, false)
 	for _, entry in ipairs(settings) do
 		if entry.type == "category" then
 			result = result .. "\tgettext(\"" .. entry.name .. "\");\n"
@@ -511,16 +710,9 @@ if false then
 end
 
 if false then
-	local file = io.open("src/settings_translation_file.c", "w")
+	local file = io.open("src/settings_translation_file.cpp", "w")
 	if file then
 		file:write(create_translation_file())
 		file:close()
 	end
 end
-
-tab_settings = {
-	name = "settings",
-	caption = fgettext("Settings"),
-	cbf_formspec = create_settings_formspec,
-	cbf_button_handler = handle_settings_buttons,
-}