minetest-mods / craftguide

:book: The most comprehensive Crafting Guide on Minetest
Other
43 stars 20 forks source link

core.register_alias and core.register_alias_force should not be redefined #92

Closed bell07 closed 4 years ago

bell07 commented 4 years ago

Mods loaded before craftguide cannot use this enhancement. If early loaded mods registers aliases, they are not in the cache table. The beter way is to use existing minetest.registered_aliases table or build the cache table from them.

Unsure how to deal with core.register_craft redefinition. Suspect not all registered crafts are handled right if loaded before craftguide.

kilbith commented 4 years ago

Good idea: https://github.com/minetest-mods/craftguide/commit/94637c624ef0d4a0ecd8a285b8c16aa43842b551

Unsure how to deal with core.register_craft redefinition. Suspect not all registered crafts are handled right if loaded before craftguide.

If not, they're handled the old way as fallback.

bell07 commented 4 years ago

Tested, does not work. Table is registered_alias[old_name] = new_name

Just for reference, in smart_inventory I handled the aliases in the next way:

Resolving the ingredients of recipe: local itemname = minetest.registered_aliases[recipe_item] or recipe_item

To build up the cache:

local function fill_recipe_cache()
    for itemname, _ in pairs(minetest.registered_items) do
        add_recipes_from_list(minetest.get_all_craft_recipes(itemname))
    end
    for itemname, _ in pairs(minetest.registered_aliases) do
        add_recipes_from_list(minetest.get_all_craft_recipes(itemname))
    end
end
kilbith commented 4 years ago

Can you give me examples to reproduce please?

bell07 commented 4 years ago

Still tested with homedecor silicon lump alias

kilbith commented 4 years ago

I see them, so I don't know see what's going wrong.

kilbith commented 4 years ago

Does that work better? https://github.com/minetest-mods/craftguide/commit/9e48e9ea3e79ecd9c2d520f3a239999f811aa661

bell07 commented 4 years ago

I expect to see this recipe: image

After https://github.com/minetest-mods/craftguide/commit/9e48e9ea3e79ecd9c2d520f3a239999f811aa661 the recipe is visible, but now I have 2x silicon lump in the list. One crafteable by the expected recipe, and one as ingredient for Solar panel and "Integrated" Chip (like before the change). Both entries needs to be merged.

kilbith commented 4 years ago

But both are different items, right? This is a misuse of aliases IMO.

bell07 commented 4 years ago

Borh are the same because of forced alias. You can craft the one and then use the result for craft recipes from other entry