New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more Recipes to Recipebook #469

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@Wilma456
Contributor

Wilma456 commented Sep 27, 2017

With a little help of @SquidDev . Just write a comment, if I had forget one. Closes #467.

@SquidDev

This comment has been minimized.

Contributor

SquidDev commented Sep 27, 2017

Looks good. A couple of things worth changing though:

  • Remove the player head recipes. I'm pretty sure these are an Easter egg, and so should be hidden.
  • Drop all the junk in advancement names. For instance, _1, computercraft and minecraft. It makes sense for the recipes to have those names as they are auto-generated, but it's nicer if the advancements have "cleaner" names.
  • I only really thought about it, but can merge all the recipes for one item into a single advancement. For instance, here's all the pocket upgrades. This is great for the dye recipes, as they are all identical.
  • For upgrades, it might be worth having the "upgrade item" (so pickaxe) as an additional criteria. Obviously this doesn't work if you have multiple recipes in an advancement.

There's also a couple of issues which will result in the advancements not loading correctly:

  • You reference a computercraft:bow upgrade, but there's no such thing.

If you enter a world, you should get some diagnostic error messages detailing what problems there are. Fix those errors and rinse and repeat until everything's fixed. It's a painful business but sadly only the first error is displayed.


On a totally unrelated note: advanved_modem. How did that get through code review? This is seriously embarrassing :/.

@Wilma456

This comment has been minimized.

Contributor

Wilma456 commented Sep 28, 2017

The Recipes are autogenerated with the completion of /recipe. I don't know, if It works without _1, computercraft or minecraft. I think, I will wait for a comment of @dan200, if I should put everything in one advancement or add the upgrade Items as criteria.

I had worked on a shooting turtle, so my script generated a Advancement for it and had forget to remove it, because it exits on my Computer and I get no error.

I will look after he recipes tomorrow and then I will remove the Shooting Turtle Recipe.

@Wilma456

This comment has been minimized.

Contributor

Wilma456 commented Sep 29, 2017

I had now removed the bow upgrade recipe. For more steps I will wait for the opinion of @dan200.

@thatcraniumguy

This comment has been minimized.

thatcraniumguy commented May 23, 2018

Can one of the admins verify this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment