Remove source defined preprocessor - #695
Merged
majcosta merged 4 commits intoAug 22, 2026
Merged
Conversation
These are compile-time build configuration, so they belong in CMakeLists.txt where the build files can see and vary them, not in a header buried in the source tree. An Opus 4.8 session lost a long hunt looking for one of them. builddefines.h is left as an empty husk for now so that every translation unit keeps its line numbering: __LINE__ is baked into each Assert() and DebugMsg() call site, so dropping the include would move thousands of immediates and hide any real code change in the noise. With the include left in place all twelve executables (JA2, JA2MAPEDITOR, JA2UB, JA2UBMAPEDITOR, symbolize_crash and Ja2Export, in Debug and RelWithDebInfo) come out bit-identical, once the two timestamps and the CodeView GUID that lld-link rewrites on every link are normalized away. The commented-out block that also lived here is gone: JA2UB, JA2UBMAPS, JA2EDITOR, JA2BETAVERSION, JA2TESTVERSION, DEBUG_ATTACKBUSY and JA113DEMO are all names CMakeLists.txt already owns, and having them appear as dead #defines in a header is what sends readers down the wrong path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same reasoning as builddefines.h: BMP_RANDOM, CALLBACKTIMER, WINDOWED_MODE and the three multiplayer switches are build configuration, and a header is the wrong place to keep them. None of the six was ever toggled from source -- they were unconditional #defines, or in WINDOWED_MODE's case keyed off _DEBUG. WINDOWED_MODE now keys off the Debug configuration rather than _DEBUG. That is the same thing in an ordinary Debug build, and fixes the asan Debug build, which links the release CRT and so never saw _DEBUG at all. All twelve executables stay bit-identical, this time with no differing bytes to explain away at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing was left in it but the include of profiler.h, and 132 translation units were including it for that alone. Fifteen files were leaning on profiler.h to drag in <set>, <vector> and <ostream> for them; those now include what they use. This is the commit that moves line numbers. Removing an include line shifts __LINE__ by one for everything below it, and __LINE__ is an immediate operand in every Assert() and DebugMsg() call, so the four game executables differ from their predecessors by roughly a thousand 32-bit constants each. Every one of those is accounted for: each is a single immediate that moved by -1 where the builddefines.h include went away, or +1 where a <set>/<vector> include was added. Nothing else in .text, .rdata or .data moves, no object file's section sizes change, and symbolize_crash and Ja2Export stay bit-identical. The one non-immediate difference is that the 24 Editor translation units of the non-editor apps stop emitting __Avx2WmemEnabledWeakValue, a 4-byte weak COMDAT they only ever instantiated through profiler.h's <vector>. It is a UCRT weak default that other translation units still provide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sgp/video.cpp guards _MT with #ifndef and then defines it 26 lines into the file, long after every header that could read it, and both cl and clang-cl already define _MT for the static runtime this project links. jascreens.cpp defines _UNICODE after its last include. The only effect on the binaries is 82 assert line numbers in video.cpp shifting by four and one in jascreens.cpp shifting by one. No object file's section sizes change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
don't set preprocessor flags BOTH in builddefines.h AND cmake, that's very confusing.
also get rid of builddefines.h