minetest-mods / craftguide

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

Progressive Mode can "miss" a couple of craft recipes #65

Closed Wuzzy2 closed 5 years ago

Wuzzy2 commented 5 years ago

How to repro:

  1. Set Progressive Mode
  2. Start MTG
  3. /giveme craftguide:book
  4. Mine tree
  5. Look in craftguide. Everything looks fine so far
  6. Craft wooden planks and immedaitely craft sticks
  7. Look in craftguide again. Whoops, where did the tool recipes go?!

Apparently the craftguide seemed to have “missed” the fact that you had wooden planks in your inventory.

You could also try this:

  1. Set Progressive Mode
  2. Start MTG
  3. /giveme craftguide:book
  4. Mine sand
  5. Throw it away
  6. Look in craftguide again. Whoops, where's the glass recipe?!

I wanna bet there are a couple of other situtations how this could happen. :-(

p-ouellette commented 5 years ago

Progressive mode currently only updates the "discovered" recipes whenever the filter is applied (when the craftguide is opened). It does this by adding undiscovered items in the player's inventory to the discovered list. The planks are never discovered because you don't open the craftguide with them in your inventory.

It would be better to "discover" an item the moment it appears in a player's inventory, but there is no Minetest callback for this. The current system could be improved by discovering items on_dignode, but this won't cover all cases so it would still be necessary to periodically check the player's inventory for undiscovered items. Doing this check when the guide is opened makes no sense because a player could find all the items in the game and put them in a chest, then open the guide with an empty inv, and they will have discovered nothing.

kilbith commented 5 years ago

We could check the current player inventory inside minetest.register_globalstep, and if an item is not inside data.inv_items: update the progressive items list according to that.

kilbith commented 5 years ago

https://github.com/minetest-mods/craftguide/commit/96c30470ccb14f2a3da4daf5a716b0127d15f5c3 Can you test please?

Wuzzy2 commented 5 years ago

Maybe also check on certain events, like actually crafting the item. ;-)

I think polling the inventory all the time is not a pretty solution, I suppose there are still ways for the mod to “miss” items. But it might be OK for a workaround.

Will test later.

What we would actually need would be an engine call to detect when the contents of the player inventory changed. This would make things much simpler.

kilbith commented 5 years ago

Maybe also check on certain events, like actually crafting the item

https://github.com/minetest-mods/craftguide/commit/0956e86d733e052d401f3c8d5ec382b6740f7109