Add support for sfinv

[stujones11]

For some reason the tabs are not always selecting properly for me, I sometimes need to click them twice to get them to highlight. I am uncertain if that has to do with this particular implementation or if this is a problem with sfinv. I am testing with a freshly pulled and rebuilt engine and game, only minutes old.

[rubenwardy]

I can't reproduce this :/ I'm using latest dev too

Maybe add print statements to get?

[stujones11]

To clarify, It seems only the armor tab is effected. It does not highlight the first time you click it, even though the page has changed. It then becomes impossible to re-select the craft page until you click the armor tab a second time.

I am currently checked into -b rubenwardy-sfinv master

[rubenwardy]

I can reproduce when not in creative mode

[stujones11]

get is being called correctly for both clicks print(player:get_player_name()) prints a valid player name on each occasion. One thing is for sure, It certainly did not like me adding a call to set_page in there :-/

[stujones11]

I can reproduce when in creative mode

Did you mean survival? It is actually the opposite for me, it seems to be working okay in creative.

[rubenwardy]

I edited that post immediately afterwards, I meant it "not in creative mode"

I can reproduce when not in creative mode

[stujones11]

This is a strange one, even if I reduce the mod to only registering a sfinv page with no other mods installed, it still does the same thing. I can only guess that it has something to do with the name 3d_armor or it being part of a modpack.

@rubenwardy any ideas?

[stujones11]

I have finally narrowed it down to soft dependency on the fire mod, at least removing that appears to fix it for some weird reason. Since sfinv has nothing to do with fire (and vice-versa) I suspect there lies a much deeper problem somewhere.

Edit: As I suspected, in a simple test mod it works fine with fire soft-dependency. I guess it must have something to do with mod load order, either way it looks like sfinv is not currently usable with this mod.

[rubenwardy]

Maybe it's due to your mod also doing register_on_player_submit_fields()? I don't see how this would be affected by mod load order, as it'll always run after sfinv (which I believe cancels it)

[stujones11]

Like I said, I even went as far as to delete the entirety of init.lua and only register the sfinv page, nothing else and all other mods in the MP disabled. Only removing the fire mod soft-dep makes it work. Very strange...

[stujones11]

@rubenwardy Here is a simple test mod that reproduces this reliably for me, could you try it and confirm this is the same for you.

invtest.zip

[stujones11]

Is anyone else willing to test this and confirm that this is a game/engine bug so I can file an issue?

If you do not wish to download the zip, the code is very simple though I am guessing that adding the fire dependency to any mod that also uses sfinv will cause the same problem.

depends.txt

sfinv?
fire?

init.lua

sfinv.register_page("invtest:test", {
    title = "test",
    get = function(self, player, context)
        return sfinv.make_formspec(player, context,
            "label[0,0;Test]", true)
    end
})

Note: You must turn off creative mode in order to reproduce this issue.

[rubenwardy]

Yeah, seems to be a MTG issue. Quite busy at the moment so unsure as to when I'll be able to work on it. It's worth filing an issue

[JustinLaw64]

I think I found part of the issue, and it lies entirely with sfinv.

My fork at https://github.com/ForbiddenJ/minetest-3d_armor attempts to add a page sfinv. I also have your invtest mod installed on my installation running Minetest 0.4.15.

I added a simple print function to sfinv and also changed a function in sfinv so that it spits out a bit of debug info.

function sfinv.print(message)
    print("[sfinv] "..message)
end

function sfinv.get_nav_fs(player, context, nav, current_idx)
    -- Only show tabs if there is more than one page
    if #nav > 1 then
        sfinv.print("current_idx: "..current_idx)
        sfinv.print("sfinv.pages_unordered: ")
        for k,v in pairs(sfinv.pages_unordered) do
            sfinv.print("  ["..tostring(k).."] "..tostring(v)..", name = "..v.name)
        end
        sfinv.print("sfinv.pages: ")
        for k,v in pairs(sfinv.pages) do
            sfinv.print("  ["..tostring(k).."] "..tostring(v))
        end
        sfinv.print("context.nav_titles: "..minetest.write_json(context.nav_titles))

        return "tabheader[0,0;tabs;" .. table.concat(nav, ",") .. ";" .. current_idx .. ";true;false]"
    else
        return ""
    end
end

When I click the armor tab this comes out:

[sfinv] current_idx: 7
[sfinv] sfinv.pages_unordered: 
[sfinv]   [1] table: 0x28c0590, name = sfinv:crafting
[sfinv]   [2] table: 0x28ca300, name = creative:all
[sfinv]   [3] table: 0x28ca6b0, name = creative:nodes
[sfinv]   [4] table: 0x28cab20, name = creative:tools
[sfinv]   [5] table: 0x28caed0, name = creative:craftitems
[sfinv]   [6] table: 0x27c1390, name = invtest:test
[sfinv]   [7] table: 0x28b7980, name = 3d_armor:armorpage
[sfinv] sfinv.pages: 
[sfinv]   [creative:nodes] table: 0x28ca6b0
[sfinv]   [3d_armor:armorpage] table: 0x28b7980
[sfinv]   [invtest:test] table: 0x27c1390
[sfinv]   [creative:tools] table: 0x28cab20
[sfinv]   [creative:all] table: 0x28ca300
[sfinv]   [creative:craftitems] table: 0x28caed0
[sfinv]   [sfinv:crafting] table: 0x28c0590
[sfinv] context.nav_titles: ["Crafting","test","Armor"]

The tab gui can only highlight what it displays in context.nav_titles, which has a length of 3, however current_idx is trying to reference tab number 7, which is outside of this length. Therefore the GUI bounces back to highlighting tab 1. It should use number 3.

With fire mod removed and armor tab clicked again, this is what I get.

[sfinv] current_idx: 3
[sfinv] sfinv.pages_unordered: 
[sfinv]   [1] table: 0x7282b70, name = sfinv:crafting
[sfinv]   [2] table: 0x7282e10, name = invtest:test
[sfinv]   [3] table: 0x727b820, name = 3d_armor:armorpage
[sfinv]   [4] table: 0x26ba5c0, name = creative:all
[sfinv]   [5] table: 0x26ba9d0, name = creative:nodes
[sfinv]   [6] table: 0x5cfeea0, name = creative:tools
[sfinv]   [7] table: 0x26bb1c0, name = creative:craftitems
[sfinv] sfinv.pages: 
[sfinv]   [creative:all] table: 0x26ba5c0
[sfinv]   [3d_armor:armorpage] table: 0x727b820
[sfinv]   [invtest:test] table: 0x7282e10
[sfinv]   [creative:tools] table: 0x5cfeea0
[sfinv]   [creative:nodes] table: 0x26ba9d0
[sfinv]   [creative:craftitems] table: 0x26bb1c0
[sfinv]   [sfinv:crafting] table: 0x7282b70
[sfinv] context.nav_titles: ["Crafting","test","Armor"]

This explains why removing fire seemingly fixes the problem.

Update: I made a pr that fixes this. https://github.com/minetest/minetest_game/pull/1602

[stujones11]

This explains why removing fire seemingly fixes the problem.

@ForbiddenJ Thank you for the explanation, it sure had me beat.

[stujones11]

All seems good now with sfiinv so I will merge now since is fixes a number of other issues, thank you all for your help with this.