Skip to content

Fix: Missing Headers#380

Merged
FlyAndNotDown merged 1 commit intoExplosionEngine:masterfrom
justforlxz:master
Jan 23, 2025
Merged

Fix: Missing Headers#380
FlyAndNotDown merged 1 commit intoExplosionEngine:masterfrom
justforlxz:master

Conversation

@justforlxz
Copy link
Contributor

In order to make lsp work properly, I need to turn off some or all of CMAKE_UNITY_BUILD. I personally recommend providing a cmake preset that does not use this option by default, and a FAST preset for developers who want to speed up compilation.

This project has CMAKE_UNITY_BUILD enabled, which results in some header
files not being introduced where they are really needed, but being
spread to the current unit through other units.
@FlyAndNotDown
Copy link
Member

I'm not sure what meaing lsp is, in my test on Windows / macOS, build is OK, and IDE lint is OK (I use CLion as IDE). I'm confused about unity build broken your what ?

By the way, qt_add_executable has some internal qt work like auto add RCC/MOC property to target. So, for qt target, it is better to use qt_add_executable. I swear that I'm not forget my AddExecutable 😅

@justforlxz
Copy link
Contributor Author

I'm not sure what meaing lsp is, in my test on Windows / macOS, build is OK, and IDE lint is OK (I use CLion as IDE). I'm confused about unity build broken your what ?

By the way, qt_add_executable has some internal qt work like auto add RCC/MOC property to target. So, for qt target, it is better to use qt_add_executable. I swear that I'm not forget my AddExecutable 😅

Haha, I was overthinking it. I have already revoked the commit.

@FlyAndNotDown FlyAndNotDown changed the title Fix some issues with CMAKE_UNITY_BUILD Fix: Missing Headers Jan 23, 2025
@FlyAndNotDown FlyAndNotDown merged commit 0dd8c16 into ExplosionEngine:master Jan 23, 2025
2 checks passed
@FlyAndNotDown
Copy link
Member

Merged, thanks for your contribution.

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.

2 participants