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

Add ExchangeClone support. #335

Merged
merged 5 commits into from
Jan 6, 2024
Merged

Conversation

ThePython10110
Copy link
Contributor

@ThePython10110 ThePython10110 commented Nov 13, 2023

My mod ExchangeClone calculates an energy value for every item based on crafting recipes. I thought it would be a good idea for it to be able to use Technic's crafting methods, but I discovered that the way Technic waits until everything was loaded before populating technic.recipes made that difficult. This is an incredibly simple addition that calls a function every time a recipe is registered.

I have tested with and without ExchangeClone enabled. I doubt it could possibly mess anything up.

In case you're wondering, zzzz_exchangeclone_crafthook is named the way it is because Minetest loads mods in reverse alphabetical order.

The function this calls can be found here in the dev branch.

@OgelGames
Copy link
Contributor

OgelGames commented Nov 13, 2023

Technic waits until everything was loaded before populating technic.recipes

This is also a problem for #328, so it would be preferable to find a way to fix the cause than add more glue code.

Switching to use minetest.register_on_mods_loaded would be enough to fix it for your use case.

@ThePython10110
Copy link
Contributor Author

ThePython10110 commented Nov 13, 2023

Switching to use minetest.register_on_mods_loaded would be enough to fix it for your use case.

I tried that first. My guess is that minetest.after doesn't run until after all minetest.register_on_mods_loaded functions (although it is just a guess).

Edit: I just checked, and that is how it works.

Using this code:

minetest.after(0.1, function(...) minetest.log("After") end)
minetest.register_on_mods_loaded(function(...) minetest.log("Loaded") end)

"Loaded" appears before "After" every time, as far as I can tell (I tried it a few times, sometimes closing Minetest in between, in case that mattered).

@OgelGames
Copy link
Contributor

OgelGames commented Nov 13, 2023

Yes, the minetest.register_on_mods_loaded functions are run before minetest.after, in the same order that they were registered.

From the documentation:

minetest.register_on_mods_loaded(function())
Called after mods have finished loading and before the media is cached or the aliases handled.

minetest.after starts after the world loads.

Now that I look at it, minetest.register_on_mods_loaded might not work, because we need the aliases to be handled.

@BuckarooBanzay
Copy link
Member

but I discovered that the way Technic waits until everything was loaded before populating technic.recipes made that difficult.

one possibility to solve this would be to intercecpt all technic.register_recipe() calls at the load-time of your mod and at the same time check what is already registered, afaik the mesecons mod does something similar, for example:

-- store old ref
local old_register_recipe = technic.register_recipe
-- overwrite with interceptor
function technic.register_recipe(typename, data)
 -- your hook
 your_custom_code()
 -- old function
 return old_register_recipe(typename, data)
end

if everyone would implement it that way it should be compatible to each other (except if the parameter count changes)

but yeah: it is just more glue, not a proper solution 🤷

@ThePython10110
Copy link
Contributor Author

ThePython10110 commented Nov 13, 2023 via email

@OgelGames
Copy link
Contributor

I'm going to have a go at fixing it, and also try to implement group support while I'm at it.

@programmerjake
Copy link
Contributor

why not just expose a list of registered recipes that's filled when technic.register_recipe is called? this allows mods to just read the list instead of having to somehow hook the function before it's called.

@BuckarooBanzay BuckarooBanzay added Enhancement New feature or request Compatibility Aims to improve compatibility with something. labels Nov 22, 2023
@ThePython10110
Copy link
Contributor Author

It would be nice if this could be resolved before I publish the next ExchangeClone release.

@S-S-X
Copy link
Member

S-S-X commented Dec 25, 2023

It would be nice if this could be resolved before I publish the next ExchangeClone release.

There's currently 2 failures for required check, currently it is not possible to merge this:

    technic/machines/register/recipes.lua:99:121: line is too long (129 > 120)
    technic/machines/register/recipes.lua:104:3: accessing undefined variable exchangeclone

First is simple: line too long, make it shorter or break into multiple lines.

Undefined variable should be fixed by adding exchangeclone here (to read_globals):

technic/.luacheckrc

Lines 22 to 32 in f1b9928

-- Mods
"default", "stairsplus",
"screwdriver", "bucket",
"digilines", "pipeworks",
"mesecon", "moretrees",
"unified_inventory", "protector",
"unifieddyes", "digiline_remote",
"drawers", "mg",
"craftguide", "i3", "mtt",
"vizlib", "mcl_sounds", "mcl_vars",
"mcl_worlds",

@@ -96,7 +96,13 @@ local function register_recipe(typename, data)
end
end

-- Checks for "zzzz_exchangeclone_crafthook" mod so that it won't crash with older versions of ExchangeClone which don't have it.
local has_exchangeclone = minetest.get_modpath("zzzz_exchangeclone_crafthook")

This comment was marked as resolved.

@ThePython10110
Copy link
Contributor Author

ThePython10110 commented Dec 25, 2023

I fixed those things (and updated the mod name to the correct one, zzzz_exchangeclone_init, although I forgot to update the comment... whatever)

Copy link
Member

@S-S-X S-S-X left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is all it takes fine for me, after all it is just simple and self contained load time only compatibility feature which can be easily cleaned up / refactored later if / when there's more mods requiring somewhat similar compatibility stuff.

@BuckarooBanzay BuckarooBanzay merged commit 8264761 into mt-mods:master Jan 6, 2024
6 checks passed
OgelGames added a commit that referenced this pull request Jan 8, 2024
@Athozus Athozus added this to the 2.0.0 milestone Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compatibility Aims to improve compatibility with something. Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants