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

math_parser: delete the shim #71339

Merged
merged 7 commits into from
Jan 31, 2024

Conversation

andrei8l
Copy link
Contributor

@andrei8l andrei8l commented Jan 29, 2024

Summary

None

Purpose of change

It's time to delete the math_parser shim

Describe the solution

Simplify the functions that needed the shim, as much as possible, then delete the shim.

This also makes the u_val functions only available through math. They were still used through { "u_val": "xxx" } only in a couple of places and I've been meaning to disable that anyway, or at least decouple it from variable access.

Also corrected a few copy-paste, scoping, and truncation errors in the u_val code.

Describe alternatives you've considered

Reverting math: we're in too deep now.

Testing

Existing test units, for the most part. Some parts of u_val were probably never tested, given the errors in the code. I left a note about it in the documentation and will help port and/or fix them if they find use.

Additional context

I'm happy to say that the temporary shim did not turn out to be a permanent solution.

This can be reviewed by commit.

@andrei8l andrei8l force-pushed the math_parser-delete-shim branch from 04bb566 to 08e3560 Compare January 29, 2024 17:12
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. NPC / Factions NPCs, AI, Speech, Factions, Ownership [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies [C++] Changes (can be) made in C++. Previously named `Code` [Markdown] Markdown issues and PRs Mechanics: Enchantments / Spells Enchantments and spells EOC: Effects On Condition Anything concerning Effects On Condition Mods: Xedra Evolved Anything to do with Xedra Evolved Mods: Mind Over Matter labels Jan 29, 2024
@github-actions github-actions bot requested a review from Maleclypse January 29, 2024 17:24
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Auto-requesting reviews from non-collaborators: @Standing-Storm

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 29, 2024
@andrei8l andrei8l force-pushed the math_parser-delete-shim branch from 08e3560 to bf2a71a Compare January 29, 2024 21:51
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 29, 2024
@Maleclypse Maleclypse merged commit 79f8306 into CleverRaven:master Jan 31, 2024
23 of 27 checks passed
@andrei8l
Copy link
Contributor Author

Thanks for merging!

@andrei8l andrei8l deleted the math_parser-delete-shim branch January 31, 2024 03:53
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 [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. EOC: Effects On Condition Anything concerning Effects On Condition [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs Mechanics: Enchantments / Spells Enchantments and spells Mods: Mind Over Matter Mods: Xedra Evolved Anything to do with Xedra Evolved Mods Issues related to mods or modding Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants