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

Cleanup & organize includes and PCH #4019

Merged
merged 2 commits into from
Mar 18, 2022

Conversation

ranisalt
Copy link
Member

Pull Request Prelude

Changes Proposed

An effort to clean up includes, reduce header file sizes and make compilation faster. Moves includes not used in .h files to .cpp and forward declares when possible.

Includes in the PCH are required by 3 or more compilation units.

@ranisalt ranisalt requested review from EPuncker, DSpeichert and nekiro and removed request for DSpeichert March 16, 2022 19:50
@soul4soul
Copy link
Contributor

Any stats on how much this improved compile time?

Includes in the PCH are required by 3 or more compilation units.

That could be worth while to include as a comment in the PCH file as future guidance.

Copy link
Member

@DSpeichert DSpeichert left a comment

Choose a reason for hiding this comment

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

Amazing!

src/otpch.h Outdated
#include <cstdint>
#include <fmt/format.h>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#include <fmt/format.h>
#include <fmt/color.h>

While <fmt/color.h> is only used in one place, it itself includes <fmt/format.h> and therefore would be sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

Additionally, colors will be available everywhere. I'll do that (or feel free if you have time)

@DSpeichert DSpeichert merged commit c359649 into otland:master Mar 18, 2022
@ranisalt ranisalt mentioned this pull request Mar 18, 2022
3 tasks
EPuncker pushed a commit to EPuncker/forgottenserver that referenced this pull request May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants