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

Eliminate some premature division #2 #76962

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

sparr
Copy link
Member

@sparr sparr commented Oct 11, 2024

Summary

None

Purpose of change

Various places in the codebase divide integers before multiplying them, resulting in premature/unnecessary/extra truncation

Describe the solution

Rearranged code to put multiplications before divisions, or converted ints to floats sooner or at all in some calculations.

Describe alternatives you've considered

Testing

Ran test suite. Playing the game with this currently. I have not managed to reproduce most of the necessary scenarios yet, though.

Additional context

This is a sequel to #64808. I am open to suggestions to do more casts to float or double sooner, rather than just rearranging integer operations, especially where the result ends up in a float or double anyway.

@github-actions github-actions bot added NPC / Factions NPCs, AI, Speech, Factions, Ownership Vehicles Vehicles, parts, mechanics & interactions [C++] Changes (can be) made in C++. Previously named `Code` Items: Containers Things that hold other things astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 11, 2024
@CLIDragon
Copy link
Contributor

Would it be possible to write a clang-tidy lint for this?

@sparr
Copy link
Member Author

sparr commented Oct 23, 2024

I think a pretty simple lint check could catch at least half of these. I haven't had much luck figuring out how to write those yet, but this might inspire me to try harder.

@Maleclypse Maleclypse merged commit acf7a34 into CleverRaven:master Oct 24, 2024
29 checks passed
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` Items: Containers Things that hold other things json-styled JSON lint passed, label assigned by github actions NPC / Factions NPCs, AI, Speech, Factions, Ownership Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants