adriengivry / Overload

3D game engine with lua scripting
https://overloadengine.org/
MIT License
1.78k stars 225 forks source link

Pre-compiled headers support #10

Closed adriengivry closed 1 year ago

adriengivry commented 5 years ago

Overload isn't using any pch system for now.

We should start using pch in our different projects to reduce compilation time.

adriengivry commented 1 year ago

We should re-evaluate this one now that we use Premake. How feasible is it? Is it necessary? Any suggestion?

litelawliet commented 1 year ago

I don't think it is necessary at all on this project. The size and number of modules (hear projects) are too low to give a substantial benefit. Also while we can have different .pch per project (but only one .pch per .cpp file) which makes it more flexible, I don't think we have enough content to use that feature. Below are my two cents as to why I think that way.

The issue in my opinion is the fact we don't have a big enough set of includes that are use everywhere. In fact, in our headers we easily find instances where we don't care about some STL includes or custom includes that we have because we already avoided unecessary includes. I dare say that the later is still the way to go, which is to only include stuff we actually need in our .cpp or .h.

Combining that fact with first I expressed earlier about the size and number of modules, I tend to say we don't need .pch files.

In addition, I've been looking to our .h and I really can't find more than 1 or 2 includes who gets repeatedly included in multiple files very often (to define). One example which had some potential are the ECS/Components include in OvEditor, they are call in 7 others files but not always with the same include count. Sometimes you'll see a .cpp who includes 7 component files, sometimes 4, so nothing repeating and constant enough. Besides those exceptionnal case where we could find something to group together if we really want to look into it, I truly don't think we can make use of .pch.

adriengivry commented 1 year ago

Fair enough, we can close this for now.