Add simple fast inventory #1179

rubenwardy · 2016-07-04T19:51:56Z

Allows mods to define tabs for the 'i' inventory, even when creative mode is off.

sfinv.register_page("mymod:crafting2", {
    title = "Crafting2",
    get = function(self, player, context, vars)
        return sfinv.make_formspec(player, context, [[
                list[current_player;craft;1.75,0.5;3,3;]
                list[current_player;craftpreview;5.75,1.5;1,1;]
                image[4.75,1.5;1,1;gui_furnace_arrow_bg.png^[transformR270]
                listring[current_player;main]
                listring[current_player;craft]
                image[0,4.75;1,1;gui_hb_bg.png]
                image[1,4.75;1,1;gui_hb_bg.png]
                image[2,4.75;1,1;gui_hb_bg.png]
                image[3,4.75;1,1;gui_hb_bg.png]
                image[4,4.75;1,1;gui_hb_bg.png]
                image[5,4.75;1,1;gui_hb_bg.png]
                image[6,4.75;1,1;gui_hb_bg.png]
                image[7,4.75;1,1;gui_hb_bg.png]
            ]], true)
    end,
    on_player_receive_fields = function(self, player, context, fields)
        print(dump(fields))
    end
})

Potential Problems

Allowing mods to inject formspec into existing pages - eg: armor mods. This could be done currently, and would work with the default layout given, but if a layout used containers then the injected content would not be shifted down. TL;DR: it is possible for mods to inject content, but it would break layout support for that page a little

Planned improvements

When this PR is merged, I'll work on:

Unified inventory support using a wrapper
Creative inventory improvements:
- Sort by description (more natural)
- Add sub sections for nodes, such as natural items / decorations / mechanics / etc
- ...

rubenwardy · 2016-07-04T23:43:52Z

mods/sfinv/init.lua

+
+sfinv.register_page("sfinv:crafting", {
+	title = "Crafting",
+	is_in_nav = function(self, player, context)


This should be removed as not needed

sofar · 2016-07-05T00:48:52Z

I've been waiting for something like this, so, I'm very positive about a change like this.

benrob0329 · 2016-07-05T04:30:09Z

+1

t4im · 2016-07-05T13:40:29Z

mods/creative/inventory.lua

+			return [[
+					size[8,8.6]
+					bgcolor[#080808BB;true]
+					background[5,5;1,1;gui_formbg.png;true]


Aren't these overwritten later with default.gui_bg .. default.gui_bg_img ..?

Background is an image which is drawn below all other elements, this background image is at (5, 5) and has size (1, 1). I imagine that default.gui_bg covers the entire window

rubenwardy · 2016-07-05T16:26:12Z

Fixed all issues except page number coloring

SmallJoker · 2016-07-05T16:32:43Z

game_api.txt

+
+* `title` - human readable page name (required)
+* `get(self, player, context, vars)` - returns a formspec string. See formspec variables. (required)
+* `is_in_nav(self, player, context)` - return true if it appears in tab header


This looks a bit confusing, how about Return true to show it in the tab header?

fixed (hopefully to your satisfaction)

Ekdohibs · 2016-07-05T16:33:40Z

I haven't tested it, but I'm very positive about the idea as well.
While you're at it, could you please do the page number coloring change, as t4im suggested?

rubenwardy · 2016-07-05T16:46:38Z

It now uses a label and colorize instead of a table.
However the text is not center aligned.
Does anyone know how to make it so?
If not, then it will have to be reverted

C1ffisme · 2016-07-05T16:50:57Z

Very useful for things like armor, etc.

We can finally get rid of inventory_plus and it's like!

Ekdohibs · 2016-07-05T16:52:01Z

@rubenwardy Hm, wouldn't there be problems with the text overlapping other buttons if it's center-aligned?

rubenwardy · 2016-07-05T16:52:43Z

@rubenwardy Hm, wouldn't there be problems with the text overlapping other buttons if it's center-aligned?

Ideally you'd be able to specify a rectangle for the text to be in, then center align it there

We can finally get rid of inventory_plus and it's like!

That's the aim!

Ekdohibs · 2016-07-05T16:53:54Z

Hm, I don't know if this is possible though. I think keeping it left-aligned is good anyway.

rubenwardy · 2016-07-05T16:54:35Z

Is using a table that much of a problem? It's a bit overkill, but at least it looks right

rubenwardy · 2016-07-05T16:57:15Z

Please note than an older version of this code has been running on the capture the flag server with no noticed performance issues. The sfinv code is only really called on player join and tab switch

paramat · 2016-07-07T16:08:00Z

I have no opinion on this so am neutral. I can't review as am useless with inventory / formspec stuff.
Luckily it seems you have the attention of 2 mtgame devs already.

0-afflatus · 2016-07-08T23:30:53Z

Great idea, don't have time to review properly.

Fixer-007 · 2016-07-10T23:04:13Z

This should be a nice thing, I'm tired of clicking via buttons in inventory plus, should be much faster and easier with tabs 👍

rubenwardy · 2016-07-11T17:52:57Z

rebased

rubenwardy · 2016-08-01T20:54:50Z

Found something weird: searching for "bone" gets no results even though bone is seen on the first page, and has bone in the description. Very weird.
Ok, bones appear if you click left after searching for it. This makes me think that the start idx isn't being reset correctly.

t4im · 2016-08-05T21:24:16Z

mods/sfinv/api.lua

+	player:set_inventory_formspec(fs)
+end
+
+minetest.register_on_joinplayer(function(player)


Wouldn't it make sense to free the contexts on leave player, to free unused memory?

tobyplowy · 2016-11-13T15:43:05Z

+1

paramat · 2016-11-14T07:14:55Z

2 weeks to freeze, want this in release?

sofar · 2016-11-14T23:46:46Z

I'd really like to get this in, I think it'll be a good step forward for mods.

rubenwardy · 2016-11-14T23:51:27Z

Does anyone have any more objections/points?

It isn't possible to modify set_inventory_formspec, the mods would have to do sfinv.enabled = false

paramat · 2016-11-15T13:35:45Z

Sorry i can't comment being useless with inventory stuff. I have no objection to the intent.
Conflict with creative?

tobyplowy · 2016-11-26T11:38:08Z

@rubenwardy @sofar @paramat Will it get there for 0.4.15?

sofar · 2016-11-26T20:32:12Z

Works ok. Did the trash icon background disappear in this PR?

sofar · 2016-11-26T20:33:22Z

I'd recommend a huge squash+merge. 37 commits is way too much. As squashed PR it's not that big

 game_api.txt                |  106 +++++++++++++++++++
 mods/creative/depends.txt   |    1 
 mods/creative/init.lua      |  237 --------------------------------------------
 mods/creative/inventory.lua |  170 +++++++++++++++++++++++++++++++
 mods/sfinv/README.md        |   21 +++
 mods/sfinv/api.lua          |  158 +++++++++++++++++++++++++++++
 mods/sfinv/depends.txt      |    1 
 mods/sfinv/init.lua         |   22 ++++
 8 files changed, 482 insertions(+), 234 deletions(-)

rubenwardy · 2016-11-26T21:00:28Z

Of course - I was leaving squash until later

rubenwardy · 2016-11-26T21:04:16Z

done

rubenwardy · 2016-11-26T21:07:16Z

readded trash background

sofar · 2016-11-26T23:39:20Z

👍

paramat · 2016-11-27T03:36:58Z

d42ae71

paramat · 2016-11-27T03:41:08Z

But,

-if creative_mode then
-	local digtime = 42
-	local caps = {times = {digtime, digtime, digtime}, uses = 0, maxlevel = 256}
+if minetest.setting_getbool("creative_mode") then
+	local digtime = 0.5
+	local caps = {times = {digtime, digtime, digtime}, uses = 0, maxlevel = 3}

Why was this changed? 42 and 256 are intentional @rubenwardy
See 0664570
Sorry i missed this.

rubenwardy · 2016-11-27T09:19:49Z

Sfinv was started before that change, so that was a rebasing error

rubenwardy · 2016-11-27T12:14:10Z

The problems with email are due to that mod using an outdated sfinv api. Will fix later

paramat · 2016-11-27T18:43:21Z

Ok.

rubenwardy reviewed Jul 4, 2016
View reviewed changes

t4im reviewed Jul 5, 2016
View reviewed changes

SmallJoker reviewed Jul 5, 2016
View reviewed changes

rubenwardy force-pushed the sfinv branch from abda744 to 987fe62 Compare July 11, 2016 17:49

rubenwardy force-pushed the sfinv branch from 3cd600a to db31847 Compare August 1, 2016 20:46

rubenwardy mentioned this pull request Aug 5, 2016

Disable sfinv if installed minetest-mods/unified_inventory#58

Merged

t4im reviewed Aug 5, 2016
View reviewed changes

Ferk mentioned this pull request Aug 14, 2016

Optional 2x2 crafting field #1242

Closed

rubenwardy force-pushed the sfinv branch from 8add7ae to 41c6392 Compare August 28, 2016 16:30

rubenwardy removed the Rebase needed label Nov 2, 2016

rubenwardy added this to the 0.4.15 milestone Nov 14, 2016

paramat added the Rebase needed label Nov 15, 2016

rubenwardy force-pushed the sfinv branch from 5bc7077 to 9f20cfd Compare November 15, 2016 13:40

rubenwardy removed the Rebase needed label Nov 15, 2016

rubenwardy force-pushed the sfinv branch from 7df384f to c1f557f Compare November 22, 2016 23:47

rubenwardy force-pushed the sfinv branch 2 times, most recently from a56089f to 1bf739e Compare November 26, 2016 21:03

Add sfinv to allow tabs to be added to the inventory

d08b3fc

rubenwardy force-pushed the sfinv branch from 1bf739e to d08b3fc Compare November 26, 2016 21:06

sofar added the One approval label Nov 26, 2016

paramat closed this Nov 27, 2016

rubenwardy deleted the sfinv branch December 5, 2016 01:52

Add simple fast inventory #1179

Add simple fast inventory #1179

Conversation

rubenwardy commented Jul 4, 2016 • edited

Potential Problems

Planned improvements

rubenwardy Jul 4, 2016

Choose a reason for hiding this comment

sofar commented Jul 5, 2016

benrob0329 commented Jul 5, 2016

t4im Jul 5, 2016

Choose a reason for hiding this comment

rubenwardy Jul 5, 2016 • edited

Choose a reason for hiding this comment

rubenwardy commented Jul 5, 2016

SmallJoker Jul 5, 2016

Choose a reason for hiding this comment

rubenwardy Jul 5, 2016

Choose a reason for hiding this comment

Ekdohibs commented Jul 5, 2016

rubenwardy commented Jul 5, 2016

C1ffisme commented Jul 5, 2016

Ekdohibs commented Jul 5, 2016

rubenwardy commented Jul 5, 2016 • edited

Ekdohibs commented Jul 5, 2016

rubenwardy commented Jul 5, 2016

rubenwardy commented Jul 5, 2016 • edited

paramat commented Jul 7, 2016

0-afflatus commented Jul 8, 2016

Fixer-007 commented Jul 10, 2016

rubenwardy commented Jul 11, 2016

rubenwardy commented Aug 1, 2016 • edited

t4im Aug 5, 2016

Choose a reason for hiding this comment

rubenwardy Aug 5, 2016

Choose a reason for hiding this comment

tobyplowy commented Nov 13, 2016

paramat commented Nov 14, 2016

sofar commented Nov 14, 2016

rubenwardy commented Nov 14, 2016

paramat commented Nov 15, 2016 • edited

tobyplowy commented Nov 26, 2016

sofar commented Nov 26, 2016

sofar commented Nov 26, 2016

rubenwardy commented Nov 26, 2016 • edited

rubenwardy commented Nov 26, 2016

rubenwardy commented Nov 26, 2016

sofar commented Nov 26, 2016

paramat commented Nov 27, 2016

paramat commented Nov 27, 2016 • edited

rubenwardy commented Nov 27, 2016

rubenwardy commented Nov 27, 2016

paramat commented Nov 27, 2016

rubenwardy commented Jul 4, 2016 •

edited

rubenwardy Jul 5, 2016 •

edited

rubenwardy commented Jul 5, 2016 •

edited

rubenwardy commented Jul 5, 2016 •

edited

rubenwardy commented Aug 1, 2016 •

edited

paramat commented Nov 15, 2016 •

edited

rubenwardy commented Nov 26, 2016 •

edited

paramat commented Nov 27, 2016 •

edited