Remove conditional from primary compile flags in game and enet makefiles #1684
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've noticed the conditional was originally added a long time ago to the primary compile flags in #117 for respecting user settings, but this causes a few problems:
The first one is that these days most environments have some set of default flags, some of which potentially hurt performance like
-fno-omit-frame-pointerand others. They also include the more conservative -O2 optimization set. As a result, users compiling the game may not realise that the primary flags (-O3 -fomit-frame-pointer -ffast-math -fno-finite-math-only) that ensure the best performance will get replaced by the ones from the environment, which will result in subpar performance (I tested and verified it myself).In Red Eclipse Flathub version, we have to explicitly set
-O3 -fomit-frame-pointer -ffast-math -fno-finite-math-onlyin the Flatpak manifest in order to make sure only the upstream specified flags are used and it's currently the only way for us to do that. This is not ideal, because it means we have to constantly monitor the makefile for any changes in the primary flags. There is an option to setCXXFLAGS= ''in order to clear the default environment flags, but that still ignores the primary makefile flags and it only utilizes the secondary ones (-Wall -fsigned-char -fno-exceptions ...) which are set in the next line.Other environments with a default set of flags will experience the same issue if they want to use the upstream makefile flags exclusively.
That's why I think setting a conditional to the primary flags is a bad solution and it should be removed. If someone wants to force their own flags instead of the upstream ones, they can still do it without needing a conditional, like setting
export CXXFLAGS="-02 -g ..."before running make.