Skip to content
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

Autolearn recipes rework cooking part 1 #37263

Conversation

Maleclypse
Copy link
Member

@Maleclypse Maleclypse commented Jan 21, 2020

Summary

SUMMARY: Content "Cooking autolearn rework part 1"

Purpose of change

Works towards fixing the autolearn bloat.

Describe the solution

Reduces number of autolearn food recipes. Add's books where needed to reduce autolearns and no existing book appropriately covers the topic.

Describe alternatives you've considered

None

Testing

TBD still needs to add spawns for new books. Need to go through recipe_food.json. Trying to decide if canning and can sealer manuals need to be added. There are 288 autolearned recipes in recipe_food.json. Pray for me and my quest to unknown Kadath.

Additional context

Suggestions welcome. The remaining json file for this portion has over 6k lines.

Maleclypse and others added 2 commits January 20, 2020 21:56
Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
Maleclypse and others added 2 commits January 20, 2020 21:57
Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
Copy link
Member

@anothersimulacrum anothersimulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could probably use a generic abstract cooking_recipe_book, and then use copy-from for each individual book for common things, such as weight or volume.

Co-Authored-By: anothersimulacrum <anothersimulacrum@gmail.com>
@Maleclypse
Copy link
Member Author

You could probably use a generic abstract cooking_recipe_book, and then use copy-from for each individual book for common things, such as weight or volume.

Can I do that to all the existing cooking books as well as my new books?

@anothersimulacrum
Copy link
Member

Yep.

@Maleclypse
Copy link
Member Author

@jkraybill Thanks for the exhaustive list it's very helpful and it appears to cover parts where I definitely ran out of steam. Plus side is several of the books you suggest already exist in this PR. Downside is that due to IRL commitments for the next two months I may be unable to address this as quickly myself. No requirements to do so but if you choose to review this PR and make commit suggestions I would appreciate it. If you decide to do so, I would suggest looking at the cooking recipe books json first to see which ones you suggest are covered. Thank you! :)

@jkraybill
Copy link
Contributor

@Maleclypse will do. What's the plan for this PR in terms of when it will exit WIP? I'll hold off on any wholesale book/recipe changes in cooking until this one is merged but hopefully that's not months.

@I-am-Erk
Copy link
Member

I-am-Erk commented Feb 10, 2020

There's no point in bringing it out of WIP before stable because it's a content freeze kinda mod anyway. I would suggest you could pick a few specific recipes and set up a parallel PR. Milling stands out for example. There's a chance of crossover so you could instead do something like a glassblowing recipe book and update of glassblowing recipe autolearn

@Maleclypse
Copy link
Member Author

It looks like this will always fail C++ and Travis until it's merged since it is changing autolearned recipes into book learned. I will likely continue to perform some work on it until 0.E goes live.

@anothersimulacrum
Copy link
Member

It will still fail once it's merged, those tests need to be updated.

@Maleclypse
Copy link
Member Author

It will still fail once it's merged, those tests need to be updated.

I will be the cause of all tests failing, this is how you make a name for yourself! :)

@Maleclypse
Copy link
Member Author

I know someone was talking about adding an Indian Cookbook so I left all Indian recipes in autolearn. if someone want's to add mexican as well that would be awesome.

@Maleclypse Maleclypse changed the title WIP Autolearn recipes rework cooking part 1 Autolearn recipes rework cooking part 1 Mar 8, 2020
@Maleclypse
Copy link
Member Author

This PR is ready as a first start on reducing autolearn in food. @anothersimulacrum the change in autolearn recipes in this is breaking the tests, I think you said this type of test correction would be a good learning spot for me? @I-am-Erk are there any other changes you want made to this before it goes? It's gotten large enough in it's changes that I think we need to assess the impact before more recipes in food are removed from autolearn. Make sure we haven't accidentally created bottlenecks. :)

anothersimulacrum added a commit to anothersimulacrum/Cataclysm-DDA that referenced this pull request Mar 13, 2020
anothersimulacrum added a commit to anothersimulacrum/Cataclysm-DDA that referenced this pull request Mar 13, 2020
@kevingranade kevingranade merged commit 7ba6452 into CleverRaven:master Apr 2, 2020
@Maleclypse Maleclypse deleted the Autolearn-Recipes-Rework-Cooking-part-1 branch April 13, 2020 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Game: Balance Balancing of (existing) in-game features. [JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants