From c3efcb3896117dccfe122cc39199e6551ebaabb8 Mon Sep 17 00:00:00 2001 From: Gregor Parzefall Date: Mon, 13 May 2024 18:57:50 +0200 Subject: [PATCH] Remove the last remaining sync. HTTP requests on the main thread --- builtin/mainmenu/content/contentdb.lua | 74 ++++++++++++----- builtin/mainmenu/content/dlg_contentdb.lua | 21 ++--- builtin/mainmenu/content/dlg_install.lua | 92 +++++++++++++++++++++- 3 files changed, 150 insertions(+), 37 deletions(-) diff --git a/builtin/mainmenu/content/contentdb.lua b/builtin/mainmenu/content/contentdb.lua index 9ed70dfff..e0479cb4c 100644 --- a/builtin/mainmenu/content/contentdb.lua +++ b/builtin/mainmenu/content/contentdb.lua @@ -182,14 +182,26 @@ function contentdb.get_package_by_id(id) end -local function get_raw_dependencies(package) - if package.type ~= "mod" then - return {} - end - if package.raw_deps then - return package.raw_deps +-- Create a coroutine from `fn` and provide results to `callback` when complete (dead). +-- Returns a resumer function. +local function make_callback_coroutine(fn, callback) + local co = coroutine.create(fn) + + local function resumer(...) + local ok, result = coroutine.resume(co, ...) + + if not ok then + error(result) + elseif coroutine.status(co) == "dead" then + callback(result) + end end + return resumer +end + + +local function get_raw_dependencies_async(package) local url_fmt = "/api/packages/%s/dependencies/?only_hard=1&protocol_version=%s&engine_version=%s" local version = core.get_version() local base_url = core.settings:get("contentdb_url") @@ -198,11 +210,25 @@ local function get_raw_dependencies(package) local http = core.get_http_api() local response = http.fetch_sync({ url = url }) if not response.succeeded then - core.log("error", "Unable to fetch dependencies for " .. package.url_part) - return + return nil + end + return core.parse_json(response.data) or {} +end + + +local function get_raw_dependencies_co(package, resumer) + if package.type ~= "mod" then + return {} + end + if package.raw_deps then + return package.raw_deps end - local data = core.parse_json(response.data) or {} + core.handle_async(get_raw_dependencies_async, package, resumer) + local data = coroutine.yield() + if not data then + return nil + end for id, raw_deps in pairs(data) do local package2 = contentdb.package_by_id[id:lower()] @@ -223,8 +249,8 @@ local function get_raw_dependencies(package) end -function contentdb.has_hard_deps(package) - local raw_deps = get_raw_dependencies(package) +local function has_hard_deps_co(package, resumer) + local raw_deps = get_raw_dependencies_co(package, resumer) if not raw_deps then return nil end @@ -239,8 +265,14 @@ function contentdb.has_hard_deps(package) end +function contentdb.has_hard_deps(package, callback) + local resumer = make_callback_coroutine(has_hard_deps_co, callback) + resumer(package, resumer) +end + + -- Recursively resolve dependencies, given the installed mods -local function resolve_dependencies_2(raw_deps, installed_mods, out) +local function resolve_dependencies_2_co(raw_deps, installed_mods, out, resumer) local function resolve_dep(dep) -- Check whether it's already installed if installed_mods[dep.name] then @@ -290,9 +322,9 @@ local function resolve_dependencies_2(raw_deps, installed_mods, out) local result = resolve_dep(dep) out[dep.name] = result if result and result.package and not result.installed then - local raw_deps2 = get_raw_dependencies(result.package) + local raw_deps2 = get_raw_dependencies_co(result.package, resumer) if raw_deps2 then - resolve_dependencies_2(raw_deps2, installed_mods, out) + resolve_dependencies_2_co(raw_deps2, installed_mods, out, resumer) end end end @@ -302,11 +334,10 @@ local function resolve_dependencies_2(raw_deps, installed_mods, out) end --- Resolve dependencies for a package, calls the recursive version. -function contentdb.resolve_dependencies(package, game) +local function resolve_dependencies_co(package, game, resumer) assert(game) - local raw_deps = get_raw_dependencies(package) + local raw_deps = get_raw_dependencies_co(package, resumer) local installed_mods = {} local mods = {} @@ -320,7 +351,7 @@ function contentdb.resolve_dependencies(package, game) end local out = {} - if not resolve_dependencies_2(raw_deps, installed_mods, out) then + if not resolve_dependencies_2_co(raw_deps, installed_mods, out, resumer) then return nil end @@ -337,6 +368,13 @@ function contentdb.resolve_dependencies(package, game) end +-- Resolve dependencies for a package, calls the recursive version. +function contentdb.resolve_dependencies(package, game, callback) + local resumer = make_callback_coroutine(resolve_dependencies_co, callback) + resumer(package, game, resumer) +end + + local function fetch_pkgs(params) local version = core.get_version() local base_url = core.settings:get("contentdb_url") diff --git a/builtin/mainmenu/content/dlg_contentdb.lua b/builtin/mainmenu/content/dlg_contentdb.lua index a975e1326..84ef96800 100644 --- a/builtin/mainmenu/content/dlg_contentdb.lua +++ b/builtin/mainmenu/content/dlg_contentdb.lua @@ -63,21 +63,12 @@ local function install_or_update_package(this, package) end local function on_confirm() - local has_hard_deps = contentdb.has_hard_deps(package) - if has_hard_deps then - local dlg = create_install_dialog(package) - dlg:set_parent(this) - this:hide() - dlg:show() - elseif has_hard_deps == nil then - local dlg = messagebox("error_checking_deps", - fgettext("Error getting dependencies for package")) - dlg:set_parent(this) - this:hide() - dlg:show() - else - contentdb.queue_download(package, package.path and contentdb.REASON_UPDATE or contentdb.REASON_NEW) - end + local dlg = create_install_dialog(package) + dlg:set_parent(this) + this:hide() + dlg:show() + + dlg:load_deps() end if package.type == "mod" and #pkgmgr.games == 0 then diff --git a/builtin/mainmenu/content/dlg_install.lua b/builtin/mainmenu/content/dlg_install.lua index 11cb0674b..0549e23be 100644 --- a/builtin/mainmenu/content/dlg_install.lua +++ b/builtin/mainmenu/content/dlg_install.lua @@ -15,7 +15,31 @@ --with this program; if not, write to the Free Software Foundation, Inc., --51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +local function is_still_visible(dlg) + local this = ui.find_by_name("install_dialog") + return this == dlg and not dlg.hidden +end + + +local function get_loading_formspec() + local ENABLE_TOUCH = core.settings:get_bool("enable_touch") + local w = ENABLE_TOUCH and 14 or 7 + + local formspec = { + "formspec_version[3]", + "size[", w, ",9.05]", + ENABLE_TOUCH and "padding[0.01,0.01]" or "position[0.5,0.55]", + "label[3,4.525;", fgettext("Loading..."), "]", + } + return table.concat(formspec) +end + + local function get_formspec(data) + if not data.has_hard_deps_ready then + return get_loading_formspec() + end + local selected_game, selected_game_idx = pkgmgr.find_by_gameid(core.settings:get("menu_last_game")) if not selected_game_idx then selected_game_idx = 1 @@ -27,15 +51,35 @@ local function get_formspec(data) game_list[i] = core.formspec_escape(game.title) end + if not data.deps_ready[selected_game_idx] and + not data.deps_loading[selected_game_idx] then + data.deps_loading[selected_game_idx] = true + + contentdb.resolve_dependencies(data.package, selected_game, function(deps) + if not is_still_visible(data.dlg) then + return + end + data.deps_ready[selected_game_idx] = deps + ui.update() + end) + end + + -- The value of `data.deps_ready[selected_game_idx]` may have changed + -- since the last if statement since `contentdb.resolve_dependencies` + -- calls the callback immediately if the dependencies are already cached. + if not data.deps_ready[selected_game_idx] then + return get_loading_formspec() + end + local package = data.package local will_install_deps = data.will_install_deps local deps_to_install = 0 local deps_not_found = 0 - data.dependencies = contentdb.resolve_dependencies(package, selected_game) + data.deps_chosen = data.deps_ready[selected_game_idx] local formatted_deps = {} - for _, dep in pairs(data.dependencies) do + for _, dep in pairs(data.deps_chosen) do formatted_deps[#formatted_deps + 1] = "#fff" formatted_deps[#formatted_deps + 1] = core.formspec_escape(dep.name) if dep.installed then @@ -128,7 +172,7 @@ local function handle_submit(this, fields) contentdb.queue_download(data.package, contentdb.REASON_NEW) if data.will_install_deps then - for _, dep in pairs(data.dependencies) do + for _, dep in pairs(data.deps_chosen) do if not dep.is_optional and not dep.installed and dep.package then contentdb.queue_download(dep.package, contentdb.REASON_DEPENDENCY) end @@ -153,10 +197,50 @@ local function handle_submit(this, fields) end +local function load_deps(dlg) + local package = dlg.data.package + + contentdb.has_hard_deps(package, function(result) + if not is_still_visible(dlg) then + return + end + + if result == nil then + local parent = dlg.parent + dlg:delete() + local dlg2 = messagebox("error_checking_deps", + fgettext("Error getting dependencies for package $1", package.url_part)) + dlg2:set_parent(parent) + parent:hide() + dlg2:show() + elseif result == false then + contentdb.queue_download(package, package.path and contentdb.REASON_UPDATE or contentdb.REASON_NEW) + dlg:delete() + else + assert(result == true) + dlg.data.has_hard_deps_ready = true + end + ui.update() + end) +end + + function create_install_dialog(package) local dlg = dialog_create("install_dialog", get_formspec, handle_submit, nil) - dlg.data.dependencies = nil + dlg.data.deps_chosen = nil dlg.data.package = package dlg.data.will_install_deps = true + + dlg.data.has_hard_deps_ready = false + dlg.data.deps_ready = {} + dlg.data.deps_loading = {} + + dlg.load_deps = load_deps + + -- `get_formspec` needs to access `dlg` to check whether it's still open. + -- It doesn't suffice to check that any "install_dialog" instance is open + -- via `ui.find_by_name`, it's necessary to check for this exact instance. + dlg.data.dlg = dlg + return dlg end