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

Replace Irrlicht file picker with new Lua dialog #13512

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions LICENSE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ erlehmann, Warr1024, rollerozxa:
textures/base/pack/no_screenshot.png

kilbith:
textures/base/pack/file.png
textures/base/pack/folder.png
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved
textures/base/pack/server_favorite.png
textures/base/pack/progress_bar.png
textures/base/pack/progress_bar_bg.png
Expand Down
21 changes: 8 additions & 13 deletions builtin/mainmenu/settings/components.lua
Original file line number Diff line number Diff line change
Expand Up @@ -192,28 +192,23 @@ function make.path(setting)
self.changed = value ~= setting.default

local fs = ("field[0,0.3;%f,0.8;%s;%s;%s]"):format(
avail_w - 3, setting.name, get_label(setting), value)
avail_w - 3, setting.name, get_label(setting), core.formspec_escape(value))
fs = fs .. ("button[%f,0.3;1.5,0.8;%s;%s]"):format(avail_w - 3, "pick_" .. setting.name, fgettext("Browse"))
fs = fs .. ("button[%f,0.3;1.5,0.8;%s;%s]"):format(avail_w - 1.5, "set_" .. setting.name, fgettext("Set"))

return fs, 1.1
end,

on_submit = function(self, fields)
local dialog_name = "dlg_path_" .. setting.name
on_submit = function(self, fields, tabview)
if fields["pick_" .. setting.name] then
local is_file = setting.type ~= "path"
core.show_path_select_dialog(dialog_name,
is_file and fgettext_ne("Select file") or fgettext_ne("Select directory"), is_file)
return true
end
if fields[dialog_name .. "_accepted"] then
local value = fields[dialog_name .. "_accepted"]
if value ~= nil then
core.settings:set(setting.name, value)
end
local dlgtype = setting.type ~= "path" and 'file' or 'folder'
local file_browser_dlg = create_file_browser_dlg(setting.name, dlgtype)
file_browser_dlg:set_parent(tabview)
tabview:hide()
file_browser_dlg:show()
return true
end

if fields["set_" .. setting.name] or fields.key_enter_field == setting.name then
local value = fields[setting.name]
if value ~= nil then
Expand Down
160 changes: 160 additions & 0 deletions builtin/mainmenu/settings/dlg_file_browser.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
-- Minetest
-- Copyright (C) 2020 Jean-Patrick Guerrero <jeanpatrick.guerrero@gmail.com>
-- Copyright (C) 2020 EvidenceBKidscode
-- Copyright (C) 2023 ROllerozxa
--
-- This program is free software; you can redistribute it and/or modify
-- it under the terms of the GNU Lesser General Public License as published by
-- the Free Software Foundation; either version 2.1 of the License, or
-- (at your option) any later version.
--
-- This program is distributed in the hope that it will be useful,
-- but WITHOUT ANY WARRANTY; without even the implied warranty of
-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-- GNU Lesser General Public License for more details.
--
-- You should have received a copy of the GNU Lesser General Public License along
-- with this program; if not, write to the Free Software Foundation, Inc.,
-- 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

local PATH = os.getenv("HOME") -- Linux & similar
or os.getenv("HOMEDRIVE") .. os.getenv("HOMEPATH") -- Windows
Copy link
Member

Choose a reason for hiding this comment

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

probably have to do this to fix the issue @grorp mentioned

Suggested change
or os.getenv("HOMEDRIVE") .. os.getenv("HOMEPATH") -- Windows
or (os.getenv("HOMEDRIVE") .. os.getenv("HOMEPATH")) -- Windows

or core.get_user_path() -- If nothing else works
sfan5 marked this conversation as resolved.
Show resolved Hide resolved

tabdata = {}
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved

local function texture(filename)
return core.formspec_escape(defaulttexturedir .. filename .. ".png")
end

local function get_dirs()
local dirs

if PATH == "\\" then -- Windows root dir
dirs = core.get_dir_list(os.getenv("HOMEDRIVE"))
else
dirs = core.get_dir_list(PATH)
end

local new = {}
for _, f in ipairs(dirs) do
if f:sub(1,1) ~= "." then
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved
new[#new + 1] = core.formspec_escape(f)
end
end

table.sort(new)
return new
end

local function make_fs(dialogdata)
local dirs = get_dirs()

local _dirs = ""

if not next(dirs) then
_dirs = "2,,"
end

for _, f in ipairs(dirs) do
local is_file = not core.is_dir(PATH .. DIR_DELIM .. f)
Copy link
Member

Choose a reason for hiding this comment

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

The filename in f is already formspec-escaped.

Copy link
Member Author

Choose a reason for hiding this comment

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

Uh... yeah?

Copy link
Member

@sfan5 sfan5 Jun 25, 2023

Choose a reason for hiding this comment

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

The point is escaped strings are not valid filesystem paths (anymore)

Copy link
Member

Choose a reason for hiding this comment

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

E.g. if you have a folder called $hello$, it's shown as a file.

Copy link
Member

Choose a reason for hiding this comment

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

this is not fixed.

_dirs = _dirs .. (is_file and "1," or "0,") .. f .. ","
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved
end

local field_label
if dialogdata.type == 'file' then
field_label = fgettext("Selected file:")
elseif dialogdata.type == 'folder' then
field_label = fgettext("Selected folder:")
end

return table.concat({
"formspec_version[4]",
"size[14,9.2]",
"image_button[0.2,0.2;0.65,0.65;", texture('up_icon'), ";updir;]",
"field[1,0.2;12,0.65;path;;", core.formspec_escape(PATH), "]",
"tablecolumns[image,",
"0=", texture('folder'), ",",
"1=", texture('file'), ",",
"2=", texture('blank'),
";text]",
"table[0.2,1.1;13.6,6;dirs;", _dirs:sub(1,-2),
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved
";", (tabdata.selected or 1), "]",
"field[0.2,8;9,0.8;select;", field_label, ";", (tabdata.filename or ""), "]",
"button[9.5,8;2,0.8;ok;", fgettext("Open"), "]",
"button[11.7,8;2,0.8;cancel;", fgettext("Cancel"), "]"
})
end

local function fields_handler(this, fields)
if fields.cancel then
this:delete()
return true

elseif fields.ok and tabdata.filename and tabdata.filename ~= "" then
local is_file = not core.is_dir(PATH .. DIR_DELIM .. tabdata.filename)

if (this.data.type == "file" and is_file)
or (this.data.type == "folder" and not is_file) then
core.settings:set(this.data.setting, PATH .. DIR_DELIM .. tabdata.filename)

this:delete()
return true
end
end

local dirs = get_dirs()
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't unconditionally be called
it's also prone to issues if the dir list changes between load time and click time
cache this in dialogdata?

Copy link
Member

Choose a reason for hiding this comment

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

not fixed


if fields.dirs then
local event, idx = fields.dirs:sub(1,3), tonumber(fields.dirs:match("%d+"))
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

didn't we have a helper to split events?

local filename = dirs[idx]
local is_file = not core.is_dir(PATH .. DIR_DELIM .. filename)

if event == "CHG" then
tabdata.filename = filename
tabdata.selected = idx

core.update_formspec(this:get_formspec())
return true
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved

elseif event == "DCL" and filename then
if not is_file then
PATH = PATH .. (PATH == DIR_DELIM and "" or DIR_DELIM) .. filename
tabdata.selected = 1
end

core.update_formspec(this:get_formspec())
return true
end

elseif fields.updir then
PATH = string.split(PATH, DIR_DELIM)
PATH[#PATH] = nil
PATH = table.concat(PATH, DIR_DELIM)
if PLATFORM ~= "Windows" then
PATH = DIR_DELIM .. PATH
end

tabdata.selected = 1

core.update_formspec(this:get_formspec())
return true

elseif fields.path then
PATH = fields.path
rollerozxa marked this conversation as resolved.
Show resolved Hide resolved
core.update_formspec(this:get_formspec())
return true
end

return false
end

-- picker_type can be either 'file' or 'folder' for picking a file respectively a folder
function create_file_browser_dlg(setting, picker_type)
local dlg = dialog_create("file_browser", make_fs, fields_handler)

dlg.data.type = picker_type
dlg.data.setting = setting

return dlg
end
1 change: 1 addition & 0 deletions builtin/mainmenu/settings/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ local path = core.get_mainmenu_path() .. DIR_DELIM .. "settings"
dofile(path .. DIR_DELIM .. "settingtypes.lua")
dofile(path .. DIR_DELIM .. "dlg_change_mapgen_flags.lua")
dofile(path .. DIR_DELIM .. "dlg_settings.lua")
dofile(path .. DIR_DELIM .. "dlg_file_browser.lua")

-- Uncomment to generate 'minetest.conf.example' and 'settings_translation_file.cpp'.
-- For RUN_IN_PLACE the generated files may appear in the 'bin' folder.
Expand Down
12 changes: 1 addition & 11 deletions doc/menu_lua_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,6 @@ GUI
* `core.set_clouds(<true/false>)`
* `core.set_topleft_text(text)`
* `core.show_keys_menu()`
* `core.show_path_select_dialog(formname, caption, is_file_select)`
* shows a path select dialog
* `formname` is base name of dialog response returned in fields
- if dialog was accepted `"_accepted"`
will be added to fieldname containing the path
- if dialog was canceled `"_cancelled"`
will be added to fieldname value is set to formname itself
* if `is_file_select` is `true`, a file and not a folder will be selected
* returns nil or selected file/folder
* `core.get_active_driver()`:
* technical name of active video driver, e.g. "opengl"
* `core.get_active_renderer()`:
Expand Down Expand Up @@ -411,8 +402,7 @@ Async
### Limitations of Async operations
* No access to global lua variables, don't even try
* Limited set of available functions
e.g. No access to functions modifying menu like `core.start`, `core.close`,
`core.show_path_select_dialog`
e.g. No access to functions modifying menu like `core.start`, `core.close`


Background music
Expand Down
1 change: 0 additions & 1 deletion src/gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ set(gui_SRCS
${CMAKE_CURRENT_SOURCE_DIR}/guiItemImage.cpp
${CMAKE_CURRENT_SOURCE_DIR}/guiKeyChangeMenu.cpp
${CMAKE_CURRENT_SOURCE_DIR}/guiPasswordChange.cpp
${CMAKE_CURRENT_SOURCE_DIR}/guiPathSelectMenu.cpp
${CMAKE_CURRENT_SOURCE_DIR}/guiScene.cpp
${CMAKE_CURRENT_SOURCE_DIR}/guiScrollBar.cpp
${CMAKE_CURRENT_SOURCE_DIR}/guiScrollContainer.cpp
Expand Down
112 changes: 0 additions & 112 deletions src/gui/guiPathSelectMenu.cpp

This file was deleted.