future of craftguide

[bell07]

The new craftguide removed support for sfinv. Now it is a Item or node, to be right-clicked ...

Simple version of mod is mtg_craftguide in mintestest_game, but there is no way to get the reaveal functionality back with this mod.

The craftguide with inventory is i3 for now. See #48

The craftguide stay on the current commit 58e45166b3fba1a9c226718e0287b9c484994592 till the issue is solved

[dacmot]

I think I'm finally starting to understand the issue at hand, how all the different mods interact and where the compatibility issues. I think I'm starting to agree with bell07 that keeping smart_sfinv is the best option, for now anyway. I'll keep familiarizing myself with the code from the various other mods to see if I can come up with a solution.

[bell07]

My current favourite idea is to fork mtg_craftguide from MTG and enhance them for revealing and missions capabilities

[bell07]

... Or just replace craftguide with mtg_craftguide? Abandon the reveal system, show all items, and and focus the gameplay experience to awards?

[dacmot]

If we do replace craftguide with mtg_craftguide and abandon the reveal system I think it should only be temporary and short lived. My position is that the crafting reveal is essential to the survival aspect of the game.

[bell07]

For temporary solution we can just keep the current craftguide and do not update them. We are in no hurry.

Played around mtg_craftguide implementing reveal compatibility. The mtg_craftguide show less recipes then current craftguide because the "digged chance" or "burneable" are not implemented.

Just the default crafting types (shape, shapeless, cooking) are implemented in mtg_craftguide, and no expandability API like craftguide.register_craft_type().

Therefore it is worsening to switch to mtg_craftguide. Or is it ok if we do not see "burneable" or "digged by chance" in inventory.

Maybe another alternative is to follow the craftguide mod, and use the craftguide book and poster instead of inventory integration? then we need a "craft a craftguide award" ...

[dacmot]

Finally taking the time to look at this more closely.

The code for craftguide is... dense. I'm not fond of the coding style, particularly the omitted parenthesis. mtg_craftguide is much simpler and easier to follow. However, as you say bell07, it doesn't offer as much in terms of features. And with mtg being on life-support, I doubt we could push for additional features to be added upstream.

I think I would prefer to keep the current craftguide. I wouldn't mind forking it even, and cherry-picking some of the bug-fixes after support for sfinv was removed. Maybe cleaning up the code a bit too.

What do you think @Lazerbeak12345 ?

[bell07]

If forking is an alternative, maybe it is beter to fork mtg_craftguide and anhance them for missed features. I ask because I prefer the code style in mtg too.

[Lazerbeak12345]

In my personal opinion, coding style shouldn't matter very much to Whynot. Two reasons:

1. This isn't a home for mods. I don't think forking is a good idea as a long-term solution. Any changes we make need to have a plan to be sent upstream.
2. Because we don't usually edit the code unless we really have to.

[dacmot]

Coding style, as long as code quality is good, doesn't matter for WN inclusion. I agree. However, in this case we are discussing replacing craftguide. If we choose not to replace it, then it is not an issue. On the other hand, we may have to deal with bugs as it is unmaintained and frozen due to sfinv support removed.

If we do fork either craftguide or mtg_craftguide, I think it should be a standalone mod that can be used by anyone. Uncoupled from WN. Craftguide_reveal could be merged with it, and thus removed from WN. This way it could possibly be taken over by someone else while WN can continue by simply referring to it. I wouldn't mind being the maintainer for now.

[bell07]

Agree. Since Whynot is not a new home, each fork is separate project without direct relation to the game. For example I maintain "laptop" mod fork, since I do not like the (incomplete) web-pages in upstream version. The "back_to_skinsdb" contains some additional forks, basically created for whynot, but with home in my personal repositories (till changes in upstream available).

I do not plan to fork craftguide byself, ether craftguide or mtg_craftguide. If no one of us is ready to fork, the solution only I see is to update craftguide and do without sfinv integration in feature.

[dacmot]

I've forked mtg_craftguide into https://github.com/dacmot/sfcraftguide. I did some refactoring but mostly everything is as-is. @bell07, you mentioned earlier playing around with mtg_craftguide. Would you mind sharing your prototype?

[bell07]

Pushed my mtg_craftguide changes into https://github.com/dacmot/sfcraftguide/compare/reveal_api?expand=1
The changes for craftguide_reveal addon into https://github.com/minetest-mods/craftguide_reveal/compare/mtg_craftguide?expand=1

[Lazerbeak12345]

I've forked mtg_craftguide into https://github.com/dacmot/sfcraftguide. I did some refactoring but mostly everything is as-is. @bell07, you mentioned earlier playing around with mtg_craftguide. Would you mind sharing your prototype?

I really like how this code looks. We should merge the repair_api thing done by bell07 and just go for it. We should be sure to treat sfcraftguide as a separate mod, and post it on contentdb and the forum accordingly.

[Lazerbeak12345]

... The mtg_craftguide show less recipes then current craftguide because the "digged chance" or "burneable" are not implemented.

Is there any chance of adding this to @dacmot's fork?

[dacmot]

Thanks! I'm still working on merging bell07's reveal functions. I'm not fond of the function overriding mechanism, mostly because it's dependent on code/file order. So I was trying to find a different way. I also wanted to try and remove the hard dependency on doc, and then gave up^H^H^H^H^H^H^H took a break :)

What's the repair_api you're talking about?

I'm sure the diggable and burnable recipes could be added. Though I wouldn't see it as blocking for a first release.

[dacmot]

OK! Thanks for kicking my butt :)

I pushed an update with bell07's reveal code. It seems to work from the brief test I made: new MTG world with sfcraftguide, doc, and doc items mods loaded manually from the MT interface.

If this is going to stand on its own, it should probably also work with plain MTG. Currently it creates an additional recipes tab. Would either of you know how to prevent mtg_craftguide from running when both are installed?

[bell07]

The way only I see to disable mtg_craftguide is to unregister the page after the mod is loaded.
Untested code but should work:

sfinv.pages["mtg_craftguide:craftguide"] = nil
for idx = #sfinv.pages_unordered, 1, -1 do
	local page = sfinv.pages_unordered[idx]
	if page.name == "mtg_craftguide:craftguide" then	
		table.remove(sfinv.pages_unordered, idx)
	end
end

[Lazerbeak12345]

I was talking about the thing bell was talking about (edit: for whatever reason this post was massively delayed)

[dacmot]

Thanks bell07. The code above worked. I put the finishing touches on the package and submitted it on ContentDB: https://content.minetest.net/packages/Duvalon/sfcraftguide/

Next step will be to create a pull request to integrate it in WNG. Stay tuned!

[dacmot]

Package has been accepted on ContentDB.

PR submitted #87. Quick playtest seems OK in brand new WNG world.