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

Force recipes to provide (sub)category #51744

Merged
merged 2 commits into from
Sep 23, 2021

Conversation

jbytheway
Copy link
Contributor

Summary

None

Purpose of change

As suggested in #50866, recipes without a subcategory are problematic because users might never find them.

Describe the solution

Require all recipe definitions to provide a category and subcategory when loading from JSON.

To make mandatory work properly, this entailed adding a was_loaded member to recipe. Recipes aren't loaded via generic_factory, but hopefully this is a good enough approximation to work for our purposes.

Describe alternatives you've considered

Adding a check at verification time.

Testing

Unit tests.

I didn't thoroughly check mods; will let the CI do that.

Additional context

@actual-nh actual-nh added [C++] Changes (can be) made in C++. Previously named `Code` Code: Debug Debugging and troubleshooting the game, also includes the debug menu Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Sep 20, 2021
@kevingranade
Copy link
Member

kevingranade commented Sep 21, 2021

It found one, but the error isn't super informative:

Mods-([slow] ~crafting_skill_gain)=> (continued from above) ERROR : Terminated:
Error: Mods-([slow] ~crafting_skill_gain)=> missing mandatory member "subcategory"

    "type": "recipe",
    "activity_level": "fake",
    "category": "CC_OTHER",
                           ^
    "skill_used": "fabrication",
    "difficulty": 5,
    "time": "180 m",

EDIT: Actually nevermind, the error matcher gets the file and line number from... somewhere?

@jbytheway jbytheway requested a review from KorGgenT as a code owner September 22, 2021 13:28
@jbytheway
Copy link
Contributor Author

Fixed that error, but because Magicylsm is tested separately from the other mods there might be more to come.

As suggested in CleverRaven#50866, require all recipe definitions to provide a
category and subcategory when loading from JSON.
@kevingranade kevingranade merged commit 34ca637 into CleverRaven:master Sep 23, 2021
@jbytheway jbytheway deleted the mandatory_categories branch September 24, 2021 01:31
@dranitski
Copy link
Contributor

dranitski commented Sep 24, 2021

found one and fixed, here is pr #51840

 DEBUG    : Error: Json error: data/mods/Aftershock/recipes/recipe_overrides.json:133:27: missing mandatory member "subcategory"

    "activity_level": "LIGHT_EXERCISE",
    "result": "foie_gras",
    "category": "CC_FOOD",
                          ^
    "skill_used": "cooking",
    "difficulty": 6,
    "charges": 1,


 FUNCTION : bool main_menu::load_character_tab(bool)
 FILE     : src/main_menu.cpp
 LINE     : 1143
 VERSION  : fe944d5

@jbytheway
Copy link
Contributor Author

Turns out that the part of the CI that was checking mod JSON data has been broken for a while, which is why we didn't see errors outside of Magiclysm in this PR. I'm attempting to fix that in #51843.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Debug Debugging and troubleshooting the game, also includes the debug menu Crafting / Construction / Recipes Includes: Uncrafting / Disassembling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants