-
Notifications
You must be signed in to change notification settings - Fork 126
refactor: modernize NULL to nullptr #1938
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
base: main
Are you sure you want to change the base?
refactor: modernize NULL to nullptr #1938
Conversation
5903988 to
af7b405
Compare
050b3f0 to
e6c28a4
Compare
149474b to
5f9990b
Compare
xezon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the CppMacros include everwhere should not be necessary.
| * DecalGeneratorClass::Set_Mesh_Transform -- sets the current mesh coordinate system * | ||
| * - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */ | ||
|
|
||
| #include <Utility/CppMacros.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These includes should not be necessary in most cases. The include should come through WWCommon already. If not, then an always.h (WWVegas) or PreRTS.h (GameEngine) include is missing somewhere. GameEngineDevice does not use PreRTS.h there is may be necessary sometimes, but I would expect not in many cases because WWVegas stuff gets always.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I removed it from files that include always.h, PreRTS.h, and BaseType.h - cleaned up 387 of 395 of them
13beb77 to
252dac3
Compare
|
Can you merge the last commit with the first, to see which includes are left over? |
|
If you interactively rebase you should be able to drop the first and last commit entirely during the rebase so you only have the wanted commits left. |
|
It depends if there are any includes left added. But it is hard to review because github has trouble showing the diff :D |
252dac3 to
fa822b2
Compare
Done |
|
This needs another pass because #1594 has added a few new NULL's |
fa822b2 to
d88acf7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I search for NULL across whole project, then it finds 20231 NULLs, but this change only touched 8724 NULLs lines. Why is there this large discrepancy?
d88acf7 to
f312de3
Compare
| { "ZoomMaxDistance", INI::parseReal, nullptr, offsetof( AudioSettings, m_zoomMaxDistance ) }, | ||
| { "ZoomSoundVolumePercentageAmount", INI::parsePercentToReal, nullptr, offsetof( AudioSettings, m_zoomSoundVolumePercentageAmount ) }, | ||
|
|
||
| { nullptr, nullptr, nullptr, NULL } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only 3 of the 4 NULLs got converted? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, because the last field is an integer. (so the last NULL should ideally be 0)
And now it touches 28936 lines when there are 20231 NULLs. How is this possible? |
253af97 to
f92b43a
Compare
|
Now 35000 lines 👀 |
It's getting close to complete, I'm actively working on it :) |
|
But why is it 35000 lines? When I search in VS Code I get 20000 hits for NULL. Are there 15000 zeros for null? |
70d4d06 to
029eaf1
Compare
refactor(core): modernize NULL to nullptr refactor(generalsMD): modernize NULL to nullptr refactor(generals): modernize NULL to nullptr build: remove unnecessary includes refactor: modernize NULL to nullptr with clang-tidy --fix-errors refactor: Convert NULL to nullptr in Tools directories via clang-tidy use a script to convert remaining NULLs to nullptrs
fix: add CppMacros.h includes for VC6 nullptr compatibility fix: add core_utility link for Babylon tool to access CppMacros.h fix: add core_utility link for buildVersionUpdate tool to access CppMacros.h fix: add core_utility link for Tools that include CppMacros.h fix: Add core_utility to DebugWindow target to resolve CppMacros.h include fix: add CppMacros.h include for nullptr support in dialog.cpp fix: add CppMacros.h include to BFISH.cpp for nullptr support
fix: change nullptr to 0 for unsigned return type in GetLine fix: change nullptr to appropriate values for non-pointer return types fix: change false to nullptr for Path* return types in AIPathfind fix: change nullptr to false for Bool return type in pathDestination fix: correct nullptr to FALSE in Bool isPatchShadowed function
ec089de to
71957f4
Compare
71957f4 to
3f53db9
Compare
22c4663 to
6a4d020
Compare
|
Ok I think we got it done :) All NULLs (other than ones inside comments or strings) are modernized. I comment out NULL definitions, and builds are passing. On main, there are 37,524 occurrences of "NULL" (grep -rw "NULL" . | wc -l) - only about 100 of them were converted to 0 or False, the rest are now nullptr. All 1,750 remaining NULLs are intentionally left.
|
Tested build and played a game successfully.
A Follow up PR can: