search

minetest-mods / craftguide

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

Can't reveal fuel items in Progressive Mode #60

Closed Wuzzy2 closed 5 years ago

Wuzzy2 commented 5 years ago

If you're in progressive mode, it seems you cannot reveal items that are only useful as fuels.

Examples in Minetest Game: Leaves, Dry Shrub.

How to repro in MTG:

Compare with “non-Progressive Mode” in which the leaves show up.

kilbith commented 5 years ago

https://github.com/minetest-mods/craftguide/commit/fc2d2e585c464fe1892cb778c0bd83b04deae2dd

Can you test please? @pauloue Check it out, too.

Wuzzy2 commented 5 years ago

Works fine, thank you!

kilbith commented 5 years ago

Unfortunately this commit causes a serious performance regression also...

p-ouellette commented 5 years ago

It would be faster if you only get the usages if it's a fuel and there are no regular recipes. But I think it would be better to either not display these recipes at all or display them in a toggleable usages mode.

kilbith commented 5 years ago

But I think it would be better to either not display these recipes at all or display them in a toggleable usages mode.

No, that sounds like a half-baked solution. Pretty sure there exists a better fix that lies in the code architecture and that does not involve any compromise.

p-ouellette commented 5 years ago

I don't consider it a compromise. Performance issues aside, I think the current behavior of falling back to usages when there are no recipes is confusing because clicking on some items gives you usages while the rest give you recipes. It's also weird that this fallback is used only for fuel items and not other items that have usages but no recipes (e.g. default:key).

kilbith commented 5 years ago

For info, this commit drastically improves the performance of get_filtered_items.

p-ouellette commented 5 years ago

It did for me too and I was really confused as to why because it doesn't seem to change anything performance critical. So I reverted it and the performance improvement was still there. I think I know what was going on. Previously there was a bug that caused duplicate items to be added to inv_items, making the craftguide slower every time I opened it. That bug was since fixed, but the inv_items meta was never cleared, so the performance remained bad. Then when testing I cleared it and the performance was fixed.

Caching usages does make a noticeable difference when there is a ridiculous amount of registered items (dreambuilder) though.

kilbith commented 5 years ago

Talking about dreambuilder, caching all recipes/fuels/usages takes 55 seconds (<0.2s with MTG only).