Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sound api as submodule #4

Closed
SwissalpS opened this issue Jan 29, 2022 · 20 comments
Closed

sound api as submodule #4

SwissalpS opened this issue Jan 29, 2022 · 20 comments

Comments

@SwissalpS
Copy link
Contributor

This breaks update systems, having sound api as a git submodule. Please can't we keep it in basic_materials or at least use it as separate mod that basic_materials depends on?

@wsor4035
Copy link
Contributor

wsor4035 commented Jan 29, 2022

This breaks update systems

no elaboration of the problem at all.......?

@BuckarooBanzay
Copy link
Member

Another solution would a declarative approach to sounds, for example:

Registering mod

minetest.register_node("some:node", {
 sound_def_key = "core_sounds_stone"
})

Sound api mod

-- opt-depends on default, mcl, etc
minetest.register_on_mods_loaded(function()
 -- go over existing definitions and the if the "sound_def_key" is set, resolve sounds according to value
 for name, def in pairs(minetest.registered_nodes) do
  if def.sound_def_key then
   -- TODO lookup stuff depending on the mod-environment
   minetest.override_item(name, { sounds = {...} })
  end
 end
end)

-- optionally: override "minetest.register_node" to catch all late registrations
old_register_node = minetest.register_node
function minetest.register_node(name, def)
 if def.sound_def_key then
  def.sounds = { ..something.. }
 end
 -- call old function
 old_register_node(name, def)
end

Not sure if that helps in this particular use-case but it is essentially all-opt-in sounds

@SwissalpS
Copy link
Contributor Author

SwissalpS commented Jan 29, 2022

This breaks update systems

no elaboration of the problem at all.......?

so far I have not come accross other mods that use submodules. Partly because none do and so update systems don't recursively update repos.
Most probably because minetest mod system is built in a way to avoid this kind of structure.
If you break it out to be useful for other mods, it makes no sense to keep it packed into a mod. Just add it as a mod like any other and make your mod dependant on it.

Other reason, explained by SX in comments to #3

Also the method BuckarooBanzay explains above should be preferable to having a submodule repo.

The more we keep to standards already in use, the less friction we run into. Also I have no idea how CDB handles submodules.

@wsor4035
Copy link
Contributor

wsor4035 commented Jan 29, 2022

so far I have not come accross other mods that use submodules. Partly because none do and so update systems don't recursively update repos.

must not looked that hard given the irc mod has a submodule and its probably the most commonly used mod on servers. Also at least a decent handful of game use submodules.

Most probably because minetest mod system is built in a way to avoid this kind of structure.

the minetest mod system doesnt even relate to git at all

If you break it out to be useful for other mods, it makes no sense to keep it packed into a mod. Just add it as a mod like any other and make your mod dependant on it.

the point is that sounds are a core part of a mod and you shouldnt need to depend on another mod just to get core functionality.

The more we keep to standards already in use, the less friction we run into.

except submodules are already in use. also, keeping to standards already in use can basically be translated to never innovate and keep doing things the same way even though there could be better alternatives.

Also I have no idea how CDB handles submodules.

it handles them perfect fine (using https).


it seems odd that one of the arguments against submodules is that it requires people to change something even thought the alternative of depending on another mod means that they will need to install that mod as well.

@SwissalpS
Copy link
Contributor Author

SwissalpS commented Jan 30, 2022

ok, I understand. I have no clue and must shut up. :D

@S-S-X
Copy link
Member

S-S-X commented Jan 30, 2022

must not looked that hard given the irc mod has a submodule and its probably the most commonly used mod on servers. Also at least a decent handful of game use submodules.

the minetest mod system doesnt even relate to git at all

I think this is probably reference to different server and update setups where you have to admit that submodules does add complexity. What you get with that added complexity either is worth it or it is not, for mod library submodules I do not have clear opinion yet as there's only few things that are installed on very few minetest engines.

the point is that sounds are a core part of a mod and you shouldnt need to depend on another mod just to get core functionality.

This point is completely invalid. Where is authority that tells what functionality should be available through mod.conf depenency and what functionality does not belong there?

except submodules are already in use.

I think not really very widely used, especially not in most dynamic mod soup environmnets.

also, keeping to standards already in use can basically be translated to never innovate and keep doing things the same way even though there could be better alternatives.

This is very true but has nothing to do with git submodules, you're drifting off topic.

it seems odd that one of the arguments against submodules is that it requires people to change something even thought the alternative of depending on another mod means that they will need to install that mod as well.

That is how dependencies normally works and to balance this a bit: take sound api as example and mod in place of submodule allows having different mod that implements same interface for different sounds while submodule is effectively hard locked to specific version. Again one way is not simply better and there's clearly not been enough use cases to clearly tell what work best.

@wsor4035
Copy link
Contributor

I think this is probably reference to different server and update setups where you have to admit that submodules does add complexity. What you get with that added complexity either is worth it or it is not, for mod library submodules I do not have clear opinion yet as there's only few things that are installed on very few minetest engines.

submodules are in fact used in roughly 1/4 of all servers......

source irc mod on servers.minetest.net/list (of which not all servers announce there mods)
(search for irc(125) - search for irc_(34, irc_commands mod)) rounds to 100
343 servers rounded up to 400
100/400 = 1/4

@S-S-X
Copy link
Member

S-S-X commented Jan 30, 2022

submodules are in fact used in roughly 1/4 of all servers......

And that's roughly how many of minetest engines running around and using git to manage stuff? I'm pretty sure that server count is completely irrelevant here.

@wsor4035
Copy link
Contributor

if thats irrelevant than frankly i dont know what sort of metrics can be given from your point of view or my point of view that are relevant.

@S-S-X
Copy link
Member

S-S-X commented Jan 30, 2022

if thats irrelevant than frankly i dont know what sort of metrics can be given from your point of view or my point of view that are relevant.

What I see as relevant is individual users (a lot more users than servers on serverlist) and servers that are actually utilizing whole workflow changes required by submodules (probably at least few servers but for sure not all of them), that's where you get most results to see how it works and what are possible troubles.

I do understand that you do like upsides of git submodule and those do make some things better but should not forget that there's also clear downsides, that's just being ignorant.

You can give meaningful metrics through providing actual discussions about submodules around minetest environments. There's been some discussion around but not really enough to tell if submodule libraries makes minetest better, worse or just brings different workflow.
Here we do have few of these with some useful stuff and some not so useful stuff but overall it is getting somewhere, just not yet sure where.

@wsor4035
Copy link
Contributor

pasted from discord/irc.

the original issue is already resolved, if you want something like change from submodules, open a issue for that. if you want improved documentation, open a issue for that

@SwissalpS
Copy link
Contributor Author

if you want something like change from submodules, open a issue for that

I thought I was doing exactly that 😕

@wsor4035
Copy link
Contributor

This breaks update systems, having sound api as a git submodule.

@wsor4035
Copy link
Contributor

wsor4035 commented Jan 30, 2022

additional documentation added https://github.com/mt-mods/basic_materials/blob/master/README.md per @S-S-X request

@S-S-X
Copy link
Member

S-S-X commented Jan 30, 2022

This breaks update systems

no elaboration of the problem at all.......?

This was supposed to be asking for clarification about actual problem / breakage:

  • What exactly is broken (some command did not work? what command, where?)
  • Any error logs? Git error messages? ContentDB error messages?

After that start looking for solutions.

@wsor4035
Copy link
Contributor

^ to follow up on the above if additional clarification is given this issue can be reopened

@S-S-X
Copy link
Member

S-S-X commented Feb 6, 2022

Got error message that fits well in this discussion I'll add it here and reopen issue for possible further discussion.
See Technic integration tests

Status: Downloaded newer image for registry.gitlab.com/minetest/minetest/server:5.2.0
2022-02-06 09:44:15: ACTION[Main]: Server: Shutting down
2022-02-06 09:44:15: ERROR[Main]: ModError: Failed to load and run script from /var/lib/minetest/.minetest/worlds/world/worldmods/basic_materials/init.lua:
2022-02-06 09:44:15: ERROR[Main]: /var/lib/minetest/.minetest/worlds/world/worldmods/basic_materials/sound_api_core/init.lua: No such file or directory
2022-02-06 09:44:15: ERROR[Main]: stack traceback:
2022-02-06 09:44:15: ERROR[Main]: 	[C]: in function 'dofile'
2022-02-06 09:44:15: ERROR[Main]: 	...est/worlds/world/worldmods/basic_materials/nodes.lua:2: in main chunk
2022-02-06 09:44:15: ERROR[Main]: 	[C]: in function 'dofile'
2022-02-06 09:44:15: ERROR[Main]: 	...test/worlds/world/worldmods/basic_materials/init.lua:11: in main chunk
Error: Process completed with exit code 1.

Things start breaking after some time has passed and when it happens people probably try to look for existing issues so here's one for logging problems.
Not so clean fix for this error at mt-mods/technic#260 but should give idea.

@S-S-X S-S-X reopened this Feb 6, 2022
@wsor4035
Copy link
Contributor

wsor4035 commented Feb 6, 2022

literal user error for not having the submodule

@wsor4035 wsor4035 closed this as completed Feb 6, 2022
@S-S-X
Copy link
Member

S-S-X commented Feb 6, 2022

literal user error for not having the submodule

There will be literal user errors like that, reopening issue was to allow discussion and to record solutions.

You have weird definition for "user error": it is not user error but regular update example for workflows to support new basic_materials version.

@ghost
Copy link

ghost commented Jun 5, 2022

Dear developers,

I am trying to update the GNU guix package for basic_materials:

Unfortunately, guix does not support fetching submodules in git repositories, and a general policy is to unbundle dependencies from projects. The solution there is to make a separate package for the sound API. I understand that you would rather use it as a submodule, but it would be easier to us (for future updates) that basic_materials used the sound api as a dependency.

Best regards,

Vivien

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants