WIP: mcl_autogroup: Replace naming hack with register_on_mods_loaded #1961

djc · 2022-01-06T14:56:25+01:00

djc commented

2022-01-06 14:56:25 +01:00

First-time contributor

mcl_autogroup previously used a naming hack to ensure that the bulk of its code loads and executes after all mods have loaded.

This hack is no longer required. minetest provides a function register_on_mods_loaded which calls a callback after all mods have loaded.

See https://github.com/minetest/minetest/pull/7411

mcl_autogroup previously used a naming hack to ensure that the bulk of its code loads and executes after all mods have loaded. This hack is no longer required. minetest provides a function `register_on_mods_loaded` which calls a callback after all mods have loaded. See https://github.com/minetest/minetest/pull/7411

djc added 1 commit 2022-01-06 14:56:32 +01:00

2c23ab6174 _mcl_autogroup: Use register_on_mods_loaded

minetest.register_on_mods_loaded calls its callback after all mods have
loaded, removing the need for the naming hack used to ensure
_mcl_autogroup loads last.

djc force-pushed on_mods_loaded from 6311583bf3 to 4b388d1e6d

2022-01-06 14:58:23 +01:00

Compare

djc force-pushed on_mods_loaded from 4b388d1e6d to 3831a03515

2022-01-06 15:10:14 +01:00

Compare

djc commented

2022-01-06 15:10:38 +01:00

First-time contributor

Apologies for the force-pushes. I saw an opportunity to tidy up the history to make it clearer what I did. This is ready to merge now.

cora commented

2022-01-06 16:38:16 +01:00

The new autogroups is a fair bit newer than that mt commit. I remember the author saying the naming hack was def necessary... Idk... @ryvnf any particular reason you didn't use on mods loaded ?

I mean... If it works it works I guess. As a test I would propose doing the opposite and forcing it to load last but iirc this was about _mCL_autogroups having to be loaded before stuff gets registered... Not sure some input would be appreciated I guess @ryvnf

The new autogroups is a fair bit newer than that mt commit. I remember the author saying the naming hack was def necessary... Idk... @ryvnf any particular reason you didn't use on mods loaded ? I mean... If it works it works I guess. As a test I would propose doing the opposite and forcing it to load *last* but iirc this was about _mCL_autogroups having to be loaded *before* stuff gets registered... Not sure some input would be appreciated I guess @ryvnf

djc commented

2022-01-06 16:50:00 +01:00

First-time contributor

According to the comments in the code _mcl_autogroup is loaded last, and that's the point of the name.

Anyway, I will wait for @ryvnf to weigh in.

According to the comments in the code `_mcl_autogroup` *is* loaded last, and that's the point of the name. Anyway, I will wait for @ryvnf to weigh in.

cora commented

2022-01-06 17:15:16 +01:00

According to the comments in the code _mcl_autogroup is loaded last, and that's the point of the name.

Oh yeah OK don't remember the details. In that case make it load before to test ^^ but yeah would be good if ryvnf was weighed in - not sure he's reading here though i will try to notify him via other means

> According to the comments in the code `_mcl_autogroup` *is* loaded last, and that's the point of the name. > Oh yeah OK don't remember the details. In that case make it load *before* to test ^^ but yeah would be good if ryvnf was weighed in - not sure he's reading here though i will try to notify him via other means

djc commented

2022-01-06 17:47:46 +01:00

First-time contributor

fwiw with this change it does load before mods that register nodes. In particular it loads before mcl_core because mcl_core depends on mcl_autogroup, and I've moved all the code into mcl_autogroup.

However the important bits of code don't actually run until after all other mods have loaded and registered their nodes, because of the use of register_on_mods_loaded.

If you take this PR and replace the calls to register_on_mods_loaded and instead immediately execute the body of the callbacks, then when you try to run the game then it immediately crashes with attempt to index local 'def' (a nil value). The same thing happens if you take the code from current master and add a dependency on _mcl_autogroup from, say, mcl_core. So that demonstrates that this code is indeed delaying until after nodes are registered before trying to do its work.

I've tested mining some things with this change and it seems to work as expected to me. By comparison if I remove the call to overwrite() (which is the main thing that is being deferred by register_on_mods_loaded), then it's not possible to mine anything at all.

I did a visual inspection of all the places where functions defined by mcl_autogroup are called, and where nodes and tools are registered, and I'm satisfied that this solution should work at least as well as the old code.

I see one case that could be a problem. mcl_enchanting calls minetest.register_tool in a register_on_mods_loaded callback (which the Minetest API documentation seems to recommend against, although it's a bit vague). This means that those tools might not be registered before mcl_autogroup's callback runs and iterates through the tools to assign tool_capabilities. However, with the previous implementation, they definitely wouldn't be registered, so I don't think this is a problem. It looks like mcl_enchanting.initialize copies tool_capabilities from the source unenchanted item anyway, so it should work either way. Either mcl_autogroup has already assigned tool_capabilties and they will be copied by mcl_enchanting.initialize, or it hasn't, and they'll be applied to the enchanted tools when mcl_autogroup iterates through the tools.

So I'm pretty confident that this is correct. But perhaps there is some corner case that I don't know to look for.

fwiw with this change it does *load* before mods that register nodes. In particular it loads before mcl_core because mcl_core depends on mcl_autogroup, and I've moved all the code into mcl_autogroup. However the important bits of code don't actually *run* until after all other mods have loaded and registered their nodes, because of the use of `register_on_mods_loaded`. If you take this PR and replace the calls to `register_on_mods_loaded` and instead immediately execute the body of the callbacks, then when you try to run the game then it immediately crashes with `attempt to index local 'def' (a nil value)`. The same thing happens if you take the code from current `master` and add a dependency on `_mcl_autogroup` from, say, `mcl_core`. So that demonstrates that this code is indeed delaying until after nodes are registered before trying to do its work. I've tested mining some things with this change and it seems to work as expected to me. By comparison if I remove the call to `overwrite()` (which is the main thing that is being deferred by `register_on_mods_loaded`), then it's not possible to mine anything at all. I did a visual inspection of all the places where functions defined by mcl_autogroup are called, and where nodes and tools are registered, and I'm satisfied that this solution should work at least as well as the old code. I see one case that could be a problem. mcl_enchanting calls `minetest.register_tool` in a `register_on_mods_loaded` callback (which the Minetest API documentation seems to recommend against, although it's a bit vague). This means that those tools *might* not be registered before mcl_autogroup's callback runs and iterates through the tools to assign `tool_capabilities`. However, with the previous implementation, they *definitely* wouldn't be registered, so I don't think this is a problem. It looks like `mcl_enchanting.initialize` copies `tool_capabilities` from the source unenchanted item anyway, so it should work either way. Either mcl_autogroup has already assigned `tool_capabilties` and they will be copied by `mcl_enchanting.initialize`, or it hasn't, and they'll be applied to the enchanted tools when `mcl_autogroup` iterates through the tools. So I'm pretty confident that this is correct. But perhaps there is some corner case that I don't know to look for.

ryvnf commented

2022-01-06 20:20:10 +01:00

Hello! I'm the author of the mcl_autogroup mod.

When initially writing the new version of mcl_autogroup I tried to replace the old naming hack with a register_on_mods_loaded callback (like this PR), but it did not work for me then. I assumed this was because minetest.override_item didn't work in the callback. But its possible that it was just me overlooking some other part of the code which would have to be updated. I didn't look more deeply into it then and continued using the naming hack which was known to work previously.

I was interested so I tried out the changes in this PR and to me it appears to work. The MineClone2 maintainers will have to test for themselves though.

If this works it is good news. The naming hack is quite fragile (since it relies on undocumented behavior of the Minetest engine). It would be great for it to be removed.

Hello! I'm the author of the mcl_autogroup mod. When initially writing the new version of mcl_autogroup I tried to replace the old naming hack with a `register_on_mods_loaded` callback (like this PR), but it did not work for me then. I assumed this was because `minetest.override_item` didn't work in the callback. But its possible that it was just me overlooking some other part of the code which would have to be updated. I didn't look more deeply into it then and continued using the naming hack which was known to work previously. I was interested so I tried out the changes in this PR and to me it appears to work. The MineClone2 maintainers will have to test for themselves though. If this works it is good news. The naming hack is quite fragile (since it relies on undocumented behavior of the Minetest engine). It would be great for it to be removed.

👍 1 ❤️ 1

cora commented

2022-01-07 11:43:03 +01:00

So I'm pretty confident that this is correct. But perhaps there is some corner case that I don't know to look for.

I'm just saying this is fixing sth that isn't really broken so would probably be good to test this thoroughly.

> So I'm pretty confident that this is correct. But perhaps there is some corner case that I don't know to look for. I'm just saying this is fixing sth that isn't really broken so would probably be good to test this thoroughly.

djc commented

2022-01-07 20:38:36 +01:00

First-time contributor

Agreed.

I've listed everything I've done to test and verify this. If there is something that might break other than what I've listed then I don't know about it.

If there is something specific you'd like me to test then let me know. Otherwise, I am dependent on MineClone 2 developers being willing to test this thoroughly.

Agreed. I've listed everything I've done to test and verify this. If there is something that might break other than what I've listed then I don't know about it. If there is something specific you'd like me to test then let me know. Otherwise, I am dependent on MineClone 2 developers being willing to test this thoroughly.

cora commented

2022-01-08 13:00:12 +01:00

probably testing all kinds of digging would be good - with/without efficiency, silk touch etc. there have been bugs before that would enable you to instamine obsidian with certain enchantments e.g. i mean from what i understand this could impact a lot of things.

👍 1

djc commented

2022-01-09 03:08:59 +01:00

First-time contributor

I've got an idea for how to test this, so marking WIP until I'm done.

djc changed title from ~~mcl_autogroup: Replace naming hack with register_on_mods_loaded~~ to WIP: mcl_autogroup: Replace naming hack with register_on_mods_loaded

2022-01-09 03:09:12 +01:00

djc added 1 commit 2022-01-09 03:09:48 +01:00

e852d0a1c4 Merge branch 'master' into on_mods_loaded

djc commented

2022-01-09 03:54:28 +01:00

First-time contributor

So, testing this turned out to be a bit easier than I expected.

Turns out that many tools are not getting their properties set correctly. All nodes are getting the correct properties set though.

I was mainly focused on getting nodes right and didn't realise that there was so much going on with tools so it's not surprising that I missed this.

I'm not sure what the problem is yet but I suspect it's to do with mcl_enchantments. I'm confident I can get this working correctly.

To test this I made two new branches:

Both add a new command /dump_autogroup, which dumps all node and tool definitions to the console. The first branch is based on the code in this PR, and the second branch is based on master.

The procedure for testing is to follow these steps for each branch:

Launch Minetest with the branch installed.
Create and launch a new world.
Wait a few seconds.
Execute /dump_autogroup in the console. This will freeze the game for about a second because it dumps a lot of data.
Exit Minetest.
Copy debug.txt to a new file, e.g. dump.txt.
Edit dump.txt to strip out everything except the dump itself. The dump starts with Registered nodes: and ends with End dump.
Strip timestamps by running sed -i -re 's/^[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}: //' dump.txt

The dump.txt produced by each branch can then be compared using diff. They should be identical. (But they aren't, yet.)

Obviously the /dump_autogroup command shouldn't be merged into master, it's just for testing.

So, testing this turned out to be a bit easier than I expected. Turns out that many tools are not getting their properties set correctly. All nodes are getting the correct properties set though. I was mainly focused on getting nodes right and didn't realise that there was so much going on with tools so it's not surprising that I missed this. I'm not sure what the problem is yet but I suspect it's to do with mcl_enchantments. I'm confident I can get this working correctly. To test this I made two new branches: * https://git.minetest.land/djc/MineClone2/src/branch/on_mods_loaded_dump * https://git.minetest.land/djc/MineClone2/src/branch/name_hack_dump Both add a new command `/dump_autogroup`, which dumps all node and tool definitions to the console. The first branch is based on the code in this PR, and the second branch is based on master. The procedure for testing is to follow these steps for each branch: 1. Launch Minetest with the branch installed. 2. Create and launch a new world. 3. Wait a few seconds. 4. Execute `/dump_autogroup` in the console. This will freeze the game for about a second because it dumps a lot of data. 5. Exit Minetest. 6. Copy `debug.txt` to a new file, e.g. `dump.txt`. 7. Edit `dump.txt` to strip out everything except the dump itself. The dump starts with `Registered nodes:` and ends with `End dump`. 8. Strip timestamps by running `sed -i -re 's/^[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}: //' dump.txt` The `dump.txt` produced by each branch can then be compared using `diff`. They should be identical. (But they aren't, yet.) Obviously the `/dump_autogroup` command shouldn't be merged into master, it's just for testing.

djc commented

2022-01-09 05:18:58 +01:00

First-time contributor

OK, I see the problem.

There are quite a few places where register_on_mods_loaded is called in MineClone 2. Most of them read data from registered nodes and/or items and cache derived data for performance. Some of them modify registered nodes and items, or create new ones, and those are the ones that are causing the problem.

There's an inherent problem with on_mods_loaded callbacks which is that if any of them modify any part of the global state, then that creates something like a race condition where the behaviour of the callbacks is dependent on the order in which they happen to be called.

So, the reason for the _mcl_autogroups naming hack is that, to work correctly, the code in _mcl_autogroups needs to be run after all other mods have loaded, but before all on_mods_loaded callbacks defined by other mods. This explains @cora 's recollection that the reason for the hack is that _mcl_autogroups has to run before some things.

I don't think this is a long-term viable solution, and I think judging by their comments @ryvnf agrees although I think it's totally understandable that they chose not to pursue it further :-).

My thinking is that, since execution order of these callbacks matters, it should be explicitly defined somewhere instead of relying on dependencies, luck and/or peculiarities of the engine to run things in the expected order.

I am erring towards the opinion that, with minetest's architecture as it stands, no individual mod should ever register a on_mods_loaded callback(1). The reason for this is that doing so invites trouble. The individual mod can't possibly know where in order its callback should run with respect to other mods, and even if it did know, then it has no way of influencing that order (except by direct dependencies, which get tangled pretty quickly if you try to use them solely to influence load order).

(1): with the exception of a single mod whose sole purpose is to register on_mods_loaded callbacks, see below.

Instead, I think any individual mods that need to iterate over global state on startup (registered_items, etc.) should provide an init function, and that it should be the responsibility of the game or modpack that bundles mods to call those init functions in the correct order.

So for example MineClone2 might have some code somewhere that does this:

minetest.register_on_mods_loaded(function()
    mcl_autogroup.init()
    mcl_experience.init()
    mcl_enchanting.init()
    tt.init()
    -- etc.
end

-- The above ordering is invented as an example
-- and might not be the correct ordering for
-- MineClone2

This would then be the only call to register_on_mods_loaded in the whole project, and would be the source of truth for what order things should run in.

This should be much easier to maintain and understand.

Any thoughts?

OK, I see the problem. There are quite a few places where `register_on_mods_loaded` is called in MineClone 2. Most of them read data from registered nodes and/or items and cache derived data for performance. Some of them *modify* registered nodes and items, or create new ones, and those are the ones that are causing the problem. There's an inherent problem with `on_mods_loaded` callbacks which is that if any of them modify any part of the global state, then that creates something like a race condition where the behaviour of the callbacks is dependent on the order in which they happen to be called. So, the reason for the `_mcl_autogroups` naming hack is that, to work correctly, the code in `_mcl_autogroups` needs to be run *after* all other mods have loaded, but *before* all `on_mods_loaded` callbacks defined by other mods. This explains @cora 's recollection that the reason for the hack is that `_mcl_autogroups` has to run before some things. I don't think this is a long-term viable solution, and I think judging by their comments @ryvnf agrees although I think it's totally understandable that they chose not to pursue it further :-). My thinking is that, since execution order of these callbacks matters, it should be explicitly defined somewhere instead of relying on dependencies, luck and/or peculiarities of the engine to run things in the expected order. I am erring towards the opinion that, with minetest's architecture as it stands, no individual mod should *ever* register a `on_mods_loaded` callback(1). The reason for this is that doing so invites trouble. The individual mod can't possibly know where in order its callback should run with respect to other mods, and even if it did know, then it has no way of influencing that order (except by direct dependencies, which get tangled pretty quickly if you try to use them solely to influence load order). (1): with the exception of a single mod whose sole purpose is to register `on_mods_loaded` callbacks, see below. Instead, I think any individual mods that need to iterate over global state on startup (`registered_items`, etc.) should provide an `init` function, and that it should be the responsibility of the game or modpack that bundles mods to call those init functions in the correct order. So for example MineClone2 might have some code somewhere that does this: ``` minetest.register_on_mods_loaded(function() mcl_autogroup.init() mcl_experience.init() mcl_enchanting.init() tt.init() -- etc. end -- The above ordering is invented as an example -- and might not be the correct ordering for -- MineClone2 ``` This would then be the *only* call to `register_on_mods_loaded` in the whole project, and would be the source of truth for what order things should run in. This should be much easier to maintain and understand. Any thoughts?

djc commented

2022-01-09 05:33:49 +01:00

First-time contributor

(To be clear I'm proposing to implement the above solution myself provided other developers aren't horrified by the idea...)

ryvnf commented

2022-01-09 13:14:14 +01:00

OK, I see the problem.

There are quite a few places where register_on_mods_loaded is called in MineClone 2. Most of them read data from registered nodes and/or items and cache derived data for performance. Some of them modify registered nodes and items, or create new ones, and those are the ones that are causing the problem.

This might be the reason it did not work for me before. I don't remember exactly what failed and made me stop pursuing it further.

Instead, I think any individual mods that need to iterate over global state on startup (registered_items, etc.) should provide an init function, and that it should be the responsibility of the game or modpack that bundles mods to call those init functions in the correct order.

A problem with your proposed solution is that it would not work for third party mods which want to do similar things. The only way I see your proposal working is to provide a mod with some form of API to register a function to be called in that function (similar to register_on_mods_loaded).

Given this problem, I would personally leave it as it is. While the naming hack certainly is one of the more ugly parts of MineClone2, it does work and has done so consistently since it was introduced almost 4 years ago. While it relies on undocumented behavior I doubt the Minetest devs would do anything to break it.

> OK, I see the problem. > > There are quite a few places where register_on_mods_loaded is called in MineClone 2. Most of them read data from registered nodes and/or items and cache derived data for performance. Some of them modify registered nodes and items, or create new ones, and those are the ones that are causing the problem. This might be the reason it did not work for me before. I don't remember exactly what failed and made me stop pursuing it further. > Instead, I think any individual mods that need to iterate over global state on startup (registered_items, etc.) should provide an init function, and that it should be the responsibility of the game or modpack that bundles mods to call those init functions in the correct order. A problem with your proposed solution is that it would not work for third party mods which want to do similar things. The only way I see your proposal working is to provide a mod with some form of API to register a function to be called in that function (similar to `register_on_mods_loaded`). Given this problem, I would personally leave it as it is. While the naming hack certainly is one of the more ugly parts of MineClone2, it does work and has done so consistently since it was introduced almost 4 years ago. While it relies on undocumented behavior I doubt the Minetest devs would do anything to break it.

djc commented

2022-01-09 13:22:05 +01:00

First-time contributor

Contradicting myself a bit here, but I don't think it would be unreasonable for third-party mods to use minetest.register_on_mods_loaded, and equally I don't think it would be unreasonable for MineClone 2 to provide APIs for third-party mods to register callbacks at various points in the process.

The problem here as I see it is that the internal mods of MineClone 2 have many interdependencies and rely on an execution order that is not really properly defined. Hopefully a third-party mod would not be interdependent with MineClone 2 in the same way, but if it was then it would absolutely need an API to register itself at the appropriate point.

Anyway, it sounds like there is a consensus against doing this work so I'll close this PR for now.

If project members would like me to start looking at this again and implementing the proposal above please let me know and/or reopen the PR and I'll be happy to do so.

Contradicting myself a bit here, but I don't think it would be unreasonable for third-party mods to use `minetest.register_on_mods_loaded`, and equally I don't think it would be unreasonable for MineClone 2 to provide APIs for third-party mods to register callbacks at various points in the process. The problem here as I see it is that the internal mods of MineClone 2 have many interdependencies and rely on an execution order that is not really properly defined. Hopefully a third-party mod would not be interdependent with MineClone 2 in the same way, but if it was then it would absolutely need an API to register itself at the appropriate point. Anyway, it sounds like there is a consensus against doing this work so I'll close this PR for now. If project members would like me to start looking at this again and implementing the proposal above please let me know and/or reopen the PR and I'll be happy to do so.

djc closed this pull request

2022-01-09 13:22:07 +01:00

erle commented

2022-01-22 14:01:30 +01:00

Minetest devs discussed this PR after I asked about it:
https://irc.minetest.net/minetest-dev/2022-01-22#i_5925812

12:20 erlehmann MineClone2/MineClone2#1961
12:20 > While the naming hack certainly is one of the more ugly parts of MineClone2, it does work and has done so consistently since it was introduced almost 4 years ago. While it relies on undocumented behavior I doubt the Minetest devs would do anything to break it.
12:21 uh, what exactly should i tell them?
12:21 (i'm pretty sure it will be broken some time)
12:35 sfan5 wasn't it you who complained that #11438 broke MCL2?
12:35 ShadowBot https://github.com/minetest/minetest/issues/11438 -- Mods: Improve mod loading priorities (2nd) by SmallJoker
12:36 erlehmann probably. but i was of the impression that there are plans to move forward with it. was i wrong?
12:37 > There's an inherent problem with on_mods_loaded callbacks which is that if any of them modify any part of the global state, then that creates something like a race condition where the behaviour of the callbacks is dependent on the order in which they happen to be called.
12:37 this race is what the leading underscore hack avoids
12:38 > the reason for the _mcl_autogroups naming hack is that, to work correctly, the code in _mcl_autogroups needs to be run after all other mods have loaded, but before all on_mods_loaded callbacks defined by other mods
12:39 to be clear, if anyone of you has any idea how to solve this cleanly, i am pretty sure mcl* games would take that solution
12:41 sfan5 have all your game mods call mcl_autogroups.do_whatever_is_needed() at the end of their body
12:42 MTDiscord table.insert(minetest.registered_on_mods_loaded, 1, ... in _mcl_autogroups?
12:42 erlehmann it modifies the dig times for all nodes and all tools, which makes it a bit hard to add that to … all minetest mods that add tools and nodes.
12:43 thankfully it is obvious if it fails, because then digging nodes does not work at all
12:44 this would all be easier if it did not have to work with non-mcl mods at all
12:45 sfan5 if this is so important why not redefine minetest.register_tool?
12:48 erlehmann uh, wouldn't this turn the problem into “how to ensure that a mod can redefine minetest.register_tool before any other mod loads”?
12:50 sfan5 I was assuming every mod that mattered could depend on mcl_autogroup
12:50 but you could certainly make this work at any point in time by 1) modifying all tools thus far and 2) redefining register_tools for all tools to come

Minetest devs discussed this PR after I asked about it: https://irc.minetest.net/minetest-dev/2022-01-22#i_5925812 > 12:20 erlehmann https://git.minetest.land/MineClone2/MineClone2/pulls/1961 > 12:20 > While the naming hack certainly is one of the more ugly parts of MineClone2, it does work and has done so consistently since it was introduced almost 4 years ago. While it relies on undocumented behavior I doubt the Minetest devs would do anything to break it. > 12:21 uh, what *exactly* should i tell them? > 12:21 (i'm pretty sure it will be broken some time) > 12:35 sfan5 wasn't it you who complained that #11438 broke MCL2? > 12:35 ShadowBot https://github.com/minetest/minetest/issues/11438 -- Mods: Improve mod loading priorities (2nd) by SmallJoker > 12:36 erlehmann probably. but i was of the impression that there are plans to move forward with it. was i wrong? > 12:37 > There's an inherent problem with on_mods_loaded callbacks which is that if any of them modify any part of the global state, then that creates something like a race condition where the behaviour of the callbacks is dependent on the order in which they happen to be called. > 12:37 this race is what the leading underscore hack avoids > 12:38 > the reason for the _mcl_autogroups naming hack is that, to work correctly, the code in _mcl_autogroups needs to be run after all other mods have loaded, but before all on_mods_loaded callbacks defined by other mods > 12:39 to be clear, if anyone of you has any idea how to solve this cleanly, i am pretty sure mcl* games would take that solution > 12:41 sfan5 have all your game mods call mcl_autogroups.do_whatever_is_needed() at the end of their body > 12:42 MTDiscord <savilli> table.insert(minetest.registered_on_mods_loaded, 1, ... in _mcl_autogroups? > 12:42 erlehmann it modifies the dig times for all nodes and all tools, which makes it a bit hard to add that to … all minetest mods that add tools and nodes. > 12:43 thankfully it is obvious if it fails, because then digging nodes does not work at all > 12:44 this would all be easier if it did not have to work with non-mcl mods at all > 12:45 sfan5 if this is so important why not redefine minetest.register_tool? > 12:48 erlehmann uh, wouldn't this turn the problem into “how to ensure that a mod can redefine minetest.register_tool before any other mod loads”? > 12:50 sfan5 I was assuming every mod that mattered could depend on mcl_autogroup > 12:50 but you could certainly make this work at any point in time by 1) modifying all tools thus far and 2) redefining register_tools for all tools to come