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

Requirements warn if loading components as tools #77144

Merged

Conversation

RenechCDDA
Copy link
Member

@RenechCDDA RenechCDDA commented Oct 19, 2024

Summary

None

Purpose of change

Describe the solution

Specifically check requirements data during finalizing data and consistency checking to see if we're missing any valid possibilities. If there's literally nothing then it will never work.

Unfortunately, there is no way to be more specific with our error messages than giving the requirements set ID.

Describe alternatives you've considered

I would love to throw errors earlier and more fatally, during loading. But we don't know at that time if the requirements are actually valid or not. Expanding the requirements doesn't happen until inline_requirements, after all requirements have been loaded. At that point we've already lost where we were in json and there's no better information to provide, so we just have to give what we can.

Testing

shot_forming group properly throws errors with C++ changes applied

00 shot, reloaded can be crafted with json changes applied

Additional context

With this, I don't actually understand how #72801 worked or why it doesn't work here. It caught at least some of these errors in the past, but why not this one?

@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Oct 19, 2024
@RenechCDDA
Copy link
Member Author

Apparently this also catches tools loaded as components

@github-actions github-actions bot added Mods Issues related to mods or modding Mechanics: Enchantments / Spells Enchantments and spells Mods: Xedra Evolved Anything to do with Xedra Evolved labels Oct 19, 2024
@github-actions github-actions bot requested a review from Maleclypse October 19, 2024 03:51
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 19, 2024
@Maleclypse Maleclypse merged commit 05c95a3 into CleverRaven:master Oct 19, 2024
24 of 27 checks passed
@RenechCDDA RenechCDDA deleted the requirements_dont_load_components_v2 branch October 20, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mechanics: Enchantments / Spells Enchantments and spells Mods: Xedra Evolved Anything to do with Xedra Evolved Mods Issues related to mods or modding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't craft 00 Shoot, reloaded.
2 participants