minetest-whynot / whynot-game

Minetest game in minetest mods collection style
GNU General Public License v3.0
14 stars 7 forks source link

Should i3 replace sfinv and smart_sfinv? #48

Closed Lazerbeak12345 closed 2 years ago

Lazerbeak12345 commented 2 years ago

For the next release I planned to consider, if https://github.com/minetest-mods/i3 is valid replacement for sfinv and smart_sfinv. ...

Originally posted by @bell07 in https://github.com/bell07/minetest-game-whynot/issues/45#issuecomment-997865752

bell07 commented 2 years ago

i3 replaces the craftguide too, so the craftguide_reveal mod needs to be replaced / ported to i3 too. The craftguide_reveal is the compatibility layer between Wuzzys doc reveal system and kilbith craftguide reveal system).

Lazerbeak12345 commented 2 years ago

Porting might be better, if even necessary. One problem I've seen with whynot is that some mods provide the same functionality, and there's no way to really showcase both. (ex there are three hunger mods in MT contentdb, hunger, sofar/stamina TenPlus1/stamina and hbhunger. There's more of them that aren't on contentdb, such as a popular one by flux)

bell07 commented 2 years ago

Agree. Then we need decide which of them we like, based on personal taste, code quality and such things like compatibility to other mods. Had already the idea to learn makefiles and implement "make xconfig" for personal whynot configuration ;-) but no time to do that. ...

bell07 commented 2 years ago

... And remember the motto "Why not" ;-) We accept change proposal unless there is a reason to not do that. So if you say the flux one is better, we need to consider if we can replace the hunger mod

bell07 commented 2 years ago

Found out the craftguide given up the support for sfinv since https://github.com/minetest-mods/craftguide/commit/42e255f07f5233502fc07ade895a2dc02d652b52 If we stay on sfinv we need stay on old craftguide, fork them or check out the new mtg_craftguide from minetest_game (and adjust craftguide_reveal).

:-/

Lazerbeak12345 commented 2 years ago

I3 has a built-in craftguide, from my testing. It doesn't support the clothing mod, or many other mods (yet). Sortof jank with useless tabs when a mod it does support is not present. Gives off prerelease vibes.

What about unified_inventory instead of smart_sinv? It also has a bulitin craftguide.

bell07 commented 2 years ago

unified_inventory was the first inventory I used in my mods collection.

The mayor issue with them : the features are controlled by player permissions. But in my setup (1 desktop + 1 laptop for my childs) the desktop player got all permissions because of "server owner" feature, and was able to do more things in inventory then the laptop player. Unsure if this issue still persists, but since my change proposals are rejected by developers, I dislike this inventory.

In second step I wrote my own "smart_inventory".

Years later, since I have less time for minetest hacking, I abadoned the smart_inventory and ported main features into smart_sfinv modpack, as enhancement for sfinv.

In consequency the right path is to replace craftguide by mtg_craftguide (mintest_game mod), but this mod does not support reveal mode and does not provide the API for craftguide_reveal integration :-(

The second way is to follow the craftguide that was pimped up into i3 mod. I consider if I can port the smart_sfinv features into i3, if I find some time

dacmot commented 2 years ago

Unified inventory seems very complex and ugly. I like i3 for its simplicity, but there are some odd choices in the layout. Specifically where you have to scroll to see everything in the player/crafting/bags area. And because of that scroll bar, the mouse scroll wheel can't be used for splitting item stacks in the crafting table (works in the inventory, which is inconsistent).

i3 seems very actively maintained at the moment. Maybe the maintainer would be open to improvements suggestions or pull requests.

Lazerbeak12345 commented 2 years ago

Perhaps.

Here's my specific concerns with i3:

dacmot commented 2 years ago

Those are very valid concerns.

Firing up a new game I noticed the lack of crafting reveal as well... shame. I consider it a must have in survival.

Lazerbeak12345 commented 2 years ago

We could make issues upstream for these, if needed. I'm just holding off a bit on doing things like that cuz I haven't heard anything from bell in awhile.

bell07 commented 2 years ago

Did not tried i3 yet.

In documentation there is an "i3_progressive_mode" setting. But we need to consider if the "craftguide_reveal" can be ported to connect the doc system reveal to the i3 progressive reveal.

You can ask upstream for changes. I cannot contribute to i3 since the developer and me have different coding styles. In other words I do not understand his code in detail.

@Lazerbeak12345 , @dacmot, are you active lua developers and can contribute to mods if needed? As I wrote already, I am leaving Minetest and do only fine-tuning before handing over to any appreciate successor.

For me the best way is to keep sfinv and get again a working inventory for this inventory. Tried to port the craftguide from smart_inventory into sfinv, but failed because of less of time to develop. If we replace sfinv, my smart_sfinv modpack gets obsolete ...

Other choice (after i3) is to go back to smart_inventory. For this step the smart_inventory needs to be updated to the last versions of compatible mods.

dacmot commented 2 years ago

Hi bell07! I'm a C++ developer, with some experience in Perl and Java. I'm not very familiar with lua, but I've been able to modify mods in the past. I do maintain two simple packages (maple and moarmour). I've also ported the 5.x client-side translation system for several of TenPlus1's mods like farming and the various mobs ones.

i3 is being developed by jp, the same guy who made craftguide. I found the i3_progressive_mode setting last night. It can be changed in the advanced/extra/all parameters... whatever it is called in English. By the author's words in the forums about craftguide: "This mod is deprecated. However, these issues have been fixed in i3 that I want people to use instead of craftguide."

Could you clarify what you mean by "doc system" ? Is it this: https://git.minetest.org/Wuzzy/minetest_doc_modpack ? That is a nice feature, although the incomplete translations leaves something to be desired. In theory, would it just be a matter of adding a help button to i3 to access the docs? Oh, nevermind. I see. There's a doc revealing that should follow the craftguide... got it.

bell07 commented 2 years ago

You got it right! Previously the smart_inventory was used and the wuzzy's doc reveal system is well integrated into smart_inventory's crafting guide. Since I moved to sfinv + craftguide, I wrote craftguide_reveal to integrate it to the previously reveal system. It was needed to get revealed items in documentation and revealed recipes in crafting guide be sync.

By the way, I learned lua with minetest, my first mod was qa_block ;-) but without the UI the mod have currently... My paid programming language SAP ABAP ;-)

Lazerbeak12345 commented 2 years ago

Right now, yes I'm an active lua developer. Just released my first two mods on contentdb awhile ago - and have been tweaking them about daily (backwards compatible ways, ofc). (ediblestuff_api and chocolatestuff)

Lol. I learned lua with minetest as well.

dacmot commented 2 years ago

Couple of things: first, doc is currently unmaintained. Wuzzy made a last release last March 30 (2021), but the next day posted it was unmaintained, outdated and may not work (??) Maybe something I could take over from a maintenance perspective, as I do find it a nice feature.

Second, although convenient, i3 provides warping to a home location, and a waypoint system that adds a text overlay with distance pointing you to a place you have been before). The waypoint system can also show you a 3D preview of the map, giving away mineral locations. I would argue that those three features go against the spirit of rule 7.

bell07 commented 2 years ago

Then we need to consider if we remove the Wuzzy's doc mods from whynot. Should not be an issue with compatibility, except the reveal system.

bell07 commented 2 years ago

I think it is decided to stay on sfinv

Lazerbeak12345 commented 3 months ago

Been dealing with i3 a lot recently. Just wanted to add additional tags i've realised.

First of all, the code is written in such a way it's near impossible (and sometimes actually impossible) for 3rd party mods to extend. (rule 4)

Second it has TONS of features. Too many to deal with. Nothing quite like homedecor, but it's toooooo much. (rule 5)

Lazerbeak12345 commented 3 months ago

Lastly, it isn't really being maintained anymore. I wouldn't count on it anyway - it's in an archival state, and though the mt-mods team has taken it on - they've made it very clear they don't plan on doing anything with it.