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

[Maxime Devos]

Hi,

I've modified this patch such that:

  * the GUI will display mods in MINETEST_MOD_PATH as well as
    mods in ~/.minetest/mods

  * the built-in installer works even if MINETEST_MOD_PATH is set

  * MINETEST_MOD_PATH can contain multiple components

Please tell if there are other issues

I'll look into the upstream procedure for submitting patches.

Leo Prikler schreef op ma 02-08-2021 om 20:47 [+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.

All three points should be addresed by the new patch.

> > > > + -- 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?

The GUI side is very important.  All mods are disabled by default,
and in the GUI you need to choose which mods to enable.  If the GUI
doesn't know about the existence of the mod, then the mod cannot be
enabled.

> > 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?

The ‘mod selecter’ (1) and installer (2) have separate graphical interfaces,
but they use some common code 'pkgmgr.lua'.  In the new patch, 'get_modpath()'
(a string) is where mods should be installed by the built-in installer,
and 'get_modpaths()' (a list of strings) is where mods can be found.
Both interfaces work with the new patch.

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part