luk3yx / minetest-flow

Yet another formspec library for ~~Minetest~~ Luanti
GNU Lesser General Public License v2.1
7 stars 2 forks source link

feature-request: render flow form inside of flow form #13

Closed Lazerbeak12345 closed 3 months ago

Lazerbeak12345 commented 11 months ago

This at first sounds like a bad idea. It might even be a bad idea - but It has a valid use case.

Scenario

Mod exists that provides acess to the form object. The form code is entirely inline. A third party mod would like to embed that form inside of their form.

Requirements

Actual real-life inspiration

I recently saw you've been making a few flow GUIs for various things. The question I asked for myself was "what is the easiest way to embed this inside of sway? (as a 3rd party mod depending on both)" Unwrapping might work - but then you'd have to worry about ctx collisions

Example API idea

-- Victim
mod_name = {}
mod_name.form = flow.make_gui(function(player, ctx)
    -- ...
end)
-- Host
local gui = flow.widgets
host_mod_name = {}
host_mod_name.form = flow.make_gui(function (player, ctx)
    return gui.HBox{
        gui.Label{ label = "asdf" },
        -- name is wip
        -- requires ctx to be able to forward event info
        -- ensures that ctx.victim_ctx is a table (and is the victim's ctx object. contains the form event info)
        -- optional third argument for that victim ctx?
        -- ctx.form == ctx.victim_ctx.form
        mod_name.form:embed_inside_form(ctx, "victim_ctx")
    }
end)

Problems

luk3yx commented 11 months ago

Problems

* the victim is not (easily) able to share a ctx object with the victim's usual ctx

  * won't work if the victim keeps some types of state outside of the ctx
  * won't work if the victim keeps the wrong type of state inside of the ctx

* form object name collision can still happen (we don't want to prefix all of the names inside the victim since that is slow and will have to be un-prefixed before every render somehow)

I don't see any way other than prefixing all the names inside the child form (though I have been tired lately so I may be missing something, apologies if this reply is hard to follow). I don't understand why they'd need to be unprefixed though? victim_ctx.form could have a metatable like this:

setmetatable({}, {
    __index = function(_, key)
        return ctx.form[prefix .. key]
    end,
    __newindex = function(_, key, value)
        ctx.form[prefix .. key] = value
    end,
})

I have no idea how slow this would be but I'm not sure if there's a better way of doing it.

Lazerbeak12345 commented 11 months ago

Oh!

I forgot about metatables.

The next concern is that the Victim tree will have to be walked over to rename everything in the DOM

Lazerbeak12345 commented 11 months ago

Concerning speed, LuaJIT optimizes metatables - if I remember correctly. Should be near O(1) time.

Walking the whole tree is at best O(n) time.

luk3yx commented 10 months ago

The next concern is that the Victim tree will have to be walked over to rename everything in the DOM

I don't think there's any way to avoid that. I've written an embed function if you were wanting to try it out:

local function create_ctx(ctx, name, prefix)
    if not ctx[name] then
        -- Proxy ctx.form access to the real form
        ctx[name] = {
            form = setmetatable({}, {
                __index = function(_, key)
                    return ctx.form[prefix .. key]
                end,
                __newindex = function(_, key, value)
                    ctx.form[prefix .. key] = value
                end,
            })
        }
    end
    return ctx[name]
end

local function wrap_callback_func(func, name, prefix)
    return function(player, ctx)
        return func(player, create_ctx(ctx, name, prefix))
    end
end

local function add_prefix(node, name, prefix)
    if node.type == "style" and node.selectors then
        -- Add prefix to style[] selectors
        for i, selector in ipairs(node.selectors) do
            node.selectors[i] = prefix .. selector
        end
    elseif node.type == "scroll_container" and node.scrollbar_name then
        node.scrollbar_name = prefix .. node.scrollbar_name
    elseif node.type == "tooltip" and node.gui_element_name then
        node.gui_element_name = prefix .. node.gui_element_name
    end

    -- Add prefix to all names
    if node.name then
        node.name = prefix .. node.name
    end

    -- Wrap callback functions
    if node.on_event then
        node.on_event = wrap_callback_func(node.on_event, name, prefix)
    end

    if node.on_quit then
        node.on_quit = wrap_callback_func(node.on_quit, name, prefix)
    end

    -- Recurse to child nodes
    for _, child in ipairs(node) do
        add_prefix(child, name, prefix)
    end
end

function form_embed(player, form, name)
    local prefix = "\2" .. name .. "\2"
    local old_get_context = flow.get_context
    local parent_ctx = old_get_context()
    local child_ctx = create_ctx(parent_ctx, name, prefix)
    flow.get_context = function() return child_ctx end
    local node = form._build(player, child_ctx)
    flow.get_context = old_get_context

    add_prefix(node, name, prefix)
    return node
end

local gui = flow.widgets
local example_form = dofile(minetest.get_modpath("flow") .. "/example.lua")
local form = flow.make_gui(function(player, ctx)
    return gui.HBox{
        gui.VBox{
            gui.Label{label = "GUI 1 (checkbox: " .. dump(ctx.a and ctx.a.form.checkbox) .. "):"},
            form_embed(player, example_form, "a"),
        },
        gui.Box{w = 0.05, h = 1, color = "grey", padding = 0.25},
        gui.VBox{
            gui.Label{label = "GUI 2 (checkbox: " .. dump(ctx.b and ctx.b.form.checkbox) .. "):"},
            form_embed(player, example_form, "b"),
        },
    }
end)

minetest.register_chatcommand("test", {
    description = "Test form_embed()",
    func = function(name)
        form:show(minetest.get_player_by_name(name))
    end,
})
Lazerbeak12345 commented 10 months ago

I'd love to. I'll get back to you after some testing

Lazerbeak12345 commented 5 months ago

After a quick review, here's my feedback on that example code.

I haven't actually run the code - that's just a review.

Otherwise, that's exactly what I had in mind.

luk3yx commented 5 months ago
* some forms won't need a prefix. Prefixes slow things down, so if the prefix name is null, I think it should just call _build and return the result verbatim.

That sounds reasonable

* the metatable should not be recreated on every function call (it's slow)

I think it's only recreated if ctx is reset?

* perhaps the function names need work? Not sure.

I agree that they could be better, I just can't think of better names

* will the internal post-render callback used by the Flow areas need to be wrapped too?

I don't think so, they don't access ctx.

Lazerbeak12345 commented 5 months ago

I was thinking, a good abstraction for this API might be to present the API as like a widget. It might look like this:

example_form:embed{
    player = player,
    prefix = "b"
}

Otherwise, since we seem to be on the same page, I'll send a PR later.

Lazerbeak12345 commented 5 months ago

The PR I made is mostly identical to your example write-up earlier.