search

minetest-mods / craftguide

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

Add tooltips for group slots #7

Closed Wuzzy2 closed 7 years ago

Wuzzy2 commented 7 years ago

This PR changes the tooltips for the slots with group items (group:whatever). E.g. if you have a group:wood in a recipe, like for the chest, the tooltips for those slots is now changed to “Any item belonging to the wood group”. AND-combined groups (e.g. group:dye,excolor_violet) are supported, too.

And lastly, the item images are adjusted for some of the groups (different “stereotypical” item images which I think are more appropriate for the group).

Wuzzy2 commented 7 years ago

Sorry, this is too much nitpicking about unimportant things to me. I don't feel like going through my code again right now.

kilbith commented 7 years ago

This is not as nitpicky and "unimportant" as the review process on e.g. Minetest Game. I'm just ensuring that the code is stylistically good and optimal (at least for me).

Wuzzy2 commented 7 years ago

Is there a style guide?

kilbith commented 7 years ago

Yes, I try to follow more or less the Lua style guidelines for the Minetest engine, that you probably know already.

kilbith commented 7 years ago

Thanks for your contribution, merged with some modifications of my own : https://github.com/minetest-mods/craftguide/commit/f2a799bdd0cefdb5557927b84f7e2475e46832f0

Preview : capture d ecran de 2016-12-03 15-10-43

And I also added the cooktime information in the tooltips : capture d ecran de 2016-12-03 16-32-37

Wuzzy2 commented 7 years ago

Kudos for merging this anyway, and for using colorized text (but doesn't that mean the mod now breaks with Minetest 0.4.14? :-( but maybe there will be a release in the next weeks).

I dislike that you dumped my original strings in favor of the poor “group(s)” pluralization workaround. My original strings were grammatically correct.

Also, the space between “time” and the colon is ungrammatical.

(Hehe, you're not the only nitpicker, after all.)

kilbith commented 7 years ago

doesn't that mean the mod now breaks with Minetest 0.4.14?

Of course it breaks, but nobody is forced to update the mod yet ;)

I dislike that you dumped my original strings in favor of the poor “group(s)” pluralization workaround.

IMO, they were not necessary (I avoid the hardcoding). There are 3 "safety nets" that ensure to display a group stereotype anyways, the last one never failing.

the space between “time” and the colon is ungrammatical.

This is a grammatical french rule. But I can change it.

kilbith commented 7 years ago

Also thanks for doing the public announcements on the forum (much appreciated), as I don't intend to return back to this place.