search

minetest-mods / craftguide

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

progressive API improvement proposal #71

Closed bell07 closed 5 years ago

bell07 commented 5 years ago

The current progressive API can filter recipes. The default implementation does filter the recipes based on inv_items table that contain the "revealed" items. The inv_items table is a part of the default progressive implementation and should be replaceable by custom logic. Since https://github.com/minetest-mods/craftguide/commit/92cf2307dbecb41e51432e9fb91bae0d636612ca the inv_items table is used for all progressive implementations that is not always right.

My propoasal is to introduce new API function to be overriden directly:

craftguide.is_revealed(itemname, player)

or same way as for recipes:

craftguide.set_item_filter(name, function(itemname, player))
craftguide.add_item_filter(name, function(itemname, player))

to wrap around inv_items, and use them in default recipe filter.

For my use-case craftguide_reveal I need only the item filter, the recipe filter is the same but with access to the doc_item revealed table instead of inv_items

kilbith commented 5 years ago

Since 92cf230 the inv_items table is used for all progressive implementations that is not always right.

No, wrong: https://github.com/minetest-mods/craftguide/commit/a076e10d16e52084b2d8b87656dbc75e028e0c6d. Though I'm not satisfied with this patch, I'm waiting @pauloue to take a look.

bell07 commented 5 years ago

Line 263 is still item_in_inv(item, data.inv_items)). The data.inv_items is used by the default progressive implementation and managed by poll_new_items chain. Hm, there is no way to disable the poll_new_items chain for custom implementation.

kilbith commented 5 years ago

If you read the code correctly you'd see this is not used by all progressive implementations but the default one.

bell07 commented 5 years ago

The function get_filtered_items is used for all progressive implementations. Therefore since https://github.com/minetest-mods/craftguide/commit/92cf2307dbecb41e51432e9fb91bae0d636612ca the data.inv_items is used partially (check if item revealed to be shown with usages) for all implementations.

p-ouellette commented 5 years ago

data.inv_items and poll_new_items are used only for the default filter. They are not used if you disable progressive mode. The use of data.inv_items in get_filtered_items was to fix #68 and the proposed API would be necessary to make this fix apply to other recipe filters, but I'm not sure if #68 is really a big enough issue to add a new API.

bell07 commented 5 years ago

I am about the case the progressive mode is enabled, but the default implementation is replaced trough craftguide.set_recipe_filter(). The Use-case is my craftguide_reveal mod. The mod does the same as the default one, but uses the reveal-tracking table from doc_items mod instead of data.inv_items. I search for a way to replace the data.inv_items in line 262 by the doc_items table. And the way to disable the poll_new_items chain since the polling is done in doc_items.

p-ouellette commented 5 years ago

If you don't want to use the default progressive mode you should disable it and use craftguide.add_recipe_filter so that poll_new_items doesn't get called and data.inv_items doesn't get saved. Progressive mode doesn't have to be enabled to use recipe filters.

bell07 commented 5 years ago

You are rigt, I see there are a lot of changes in code since I get the deeper look last time. If I disable the progressive mode I get the issue #68 with my implementation. In #74 is my proposal to finally fix the issue. Just pass all needed informations to the filter function, the filter function can handle the crafting/usage difference.

bell07 commented 5 years ago

The reason for this issue is solved in #75. Therefore this issue can be closed if the #75 is merged.

The initially proposed "Item filter" would be still useful, but not a must-have.

If interessted I can implement the "Item filter" API functions, I just don't want to start coding in assumption it will be rejected from the beginning.