Subject: [bug#49828] [PATCH 02/20] gnu: minetest: Search for mods in MINETEST_MOD_PATH.

From: Leo Prikler <[email-masked]>
Date: Mon, 02 Aug 2021 20:47:37 +0200

Am Montag, den 02.08.2021, 19:53 +0200 schrieb Maxime Devos:
> > > +This patch as-is is not yet ready for upstream, because:
> > > +
> > > +  * the GUI will only display mods in MINETEST_MOD_PATH
> > > +    or mods in ~/.minetest/mods, it won't combine the two
> > > +
> > > +  * the GUI for installing mods from ContentDB is disabled
> > > +    when MINETEST_MOD_PATH is set, because otherwise Minetest
> > > +    would try to install mods in the store.
> > These two are fine for a "Guix-only" patch, although I do think we
> > should still read ~/.minetest/mods for backwards compatibility.
> 
> ~/.minetest/mods is still read when MINETEST_MOD_PATH is unset.
> MINETEST_MOD_PATH is only set when some mod is actually installed.
> So backwards compatibility should be ok.
I mean in the sense of "have some mods in ~/.minetest/mods, that aren't
yet packaged in Guix and have the rest sit in the profile where they
belong".  Not everyone will like the the import to manifest approach
that is needed while you're waiting for review.
> > > +  * MINETEST_MOD_PATH can only have a single component
> > This one seems kinda arbitrary, though, and does not fit well with
> > MINETEST_SUBGAME_PATH.
> 
> Yes, I know.  I didn't know how to adjust pkgmgr.lua and
> dlg_contentstore.lua
> to support multiple components, though I have an idea now to try.
> 
> > > + -- Unordered preserves the original order of the ContentDB API,
> > > + -- before the package list is ordered based on installed state.
> > > +diff --git a/src/content/subgames.cpp b/src/content/subgames.cpp
> > > +index e9dc609b0..1809f189e 100644
> > > +--- a/src/content/subgames.cpp
> > > ++++ b/src/content/subgames.cpp
> > > +@@ -110,6 +110,9 @@ SubgameSpec findSubgame(const std::string
> > > &id)
> > > + 	std::set<std::string> mods_paths;
> > > + 	if (!user_game)
> > > + 		mods_paths.insert(share + DIR_DELIM + "mods");
> > > ++	const char *env_mod_path = getenv("MINETEST_MOD_PATH");
> > > ++	if (env_mod_path)
> > > ++		mods_paths.insert(std::string(env_mod_path));
> > Here, I would instead use an std::istringstream together with
> > std::getline(<>, <>, ':') to get the components of
> > MINETEST_MOD_PATH
> > and insert each of them.  Either that or copy whatever is used for
> > MINETEST_SUBGAME_PATH.
> 
> Minetest has a class 'Strfnd' supporting iteration.  Using that,
> it should be easy to allow MINETEST_MOD_PATH to contain ":"
> seperators.
> However, the GUI client code (pkgmgr.lua)
> uses some other logic for determining which mods exists, and
> currently,
> that logic does not support ":" separators.
Hmm, how important is the GUI side here?  Can we sneek mods into it?

> > > +diff --git a/src/script/lua_api/l_mainmenu.cpp
> > > b/src/script/lua_api/l_mainmenu.cpp
> > > +index ad00de1c4..737550c42 100644
> > > +--- a/src/script/lua_api/l_mainmenu.cpp
> > > ++++ b/src/script/lua_api/l_mainmenu.cpp
> > > +@@ -495,9 +495,19 @@ int
> > > ModApiMainMenu::l_get_user_path(lua_State
> > > *L)
> > > +
> > > /****************************************************************
> > > ****
> > > **********/
> > > + int ModApiMainMenu::l_get_modpath(lua_State *L)
> > > + {
> > > ++	const char *c_modpath = getenv("MINETEST_MOD_PATH");
> > > + 	std::string modpath = fs::RemoveRelativePathComponents(
> > > + 		porting::path_user + DIR_DELIM + "mods" +
> > > DIR_DELIM);
> > > +-	lua_pushstring(L, modpath.c_str());
> > > ++	if (c_modpath == NULL)
> > > ++		c_modpath = modpath.c_str();
> > > ++	lua_pushstring(L, c_modpath);
> > > ++	return 1;
> > > ++}
> > > ++
> > > ++/**************************************************************
> > > ****
> > > ************/
> > > ++int ModApiMainMenu::l_mod_path_set(lua_State *L)
> > > ++{
> > > ++	lua_pushboolean(L, NULL !=
> > > getenv("MINETEST_MOD_PATH"));
> > > + 	return 1;
> > > + }
> > > + 
> > > +@@ -855,6 +865,7 @@ void ModApiMainMenu::Initialize(lua_State
> > > *L,
> > > int top)
> > > + 	API_FCT(get_mapgen_names);
> > > + 	API_FCT(get_user_path);
> > > + 	API_FCT(get_modpath);
> > > ++	API_FCT(mod_path_set);
> > > + 	API_FCT(get_clientmodpath);
> > > + 	API_FCT(get_gamepath);
> > > + 	API_FCT(get_texturepath);
> > > +@@ -888,6 +899,7 @@ void
> > > ModApiMainMenu::InitializeAsync(lua_State
> > > *L, int top)
> > > + 	API_FCT(get_mapgen_names);
> > > + 	API_FCT(get_user_path);
> > > + 	API_FCT(get_modpath);
> > > ++	API_FCT(mod_path_set);
> > > + 	API_FCT(get_clientmodpath);
> > > + 	API_FCT(get_gamepath);
> > > + 	API_FCT(get_texturepath);
> > > +diff --git a/src/script/lua_api/l_mainmenu.h
> > > b/src/script/lua_api/l_mainmenu.h
> > > +index ec2d20da2..719c26077 100644
> > > +--- a/src/script/lua_api/l_mainmenu.h
> > > ++++ b/src/script/lua_api/l_mainmenu.h
> > > +@@ -112,6 +112,8 @@ class ModApiMainMenu: public ModApiBase
> > > + 
> > > + 	static int l_get_modpath(lua_State *L);
> > > + 
> > > ++	static int l_mod_path_set(lua_State *L);
> > > ++
> > > + 	static int l_get_clientmodpath(lua_State *L);
> > > + 
> > > + 	static int l_get_gamepath(lua_State *L);
> > > +-- 
> > > +2.32.0
> > What are these modpaths used for?  For mod installation or for
> > querying mod existence?  If it's the former, you could leave them
> > as-is, similar to how elpa stays enabled in Emacs.
> 
> It is only used by the GUI.  The GUI looks in the directory returned
> by "get_modpath" for two things:
> 
> (1) to determine which mods exist (and to find their descriptions,  
>     their dependency list, screenshots ...).  Only the mods that 
>     exist there can be enabled.
> 
> (2) to determine where mods must be installed when using Minetest's
>     built-in installer.
I see.  Is this the same GUI for all parts or different GUIs?  I think
if we can handle the world creation parts, everything should be fine,
no?

Regards,