unify(common-ini): Move shared INI loaders to Core#2395
unify(common-ini): Move shared INI loaders to Core#2395OmarAglan wants to merge 4 commits intoTheSuperHackers:mainfrom
Conversation
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/Common/INI/INIMultiplayer.cpp | New Core file replacing both Generals and GeneralsMD stubs; implements all three multiplayer parsers, but parseMultiplayerStartingMoneyChoiceDefinition is now active for Generals (no #ifdef RTS_ZEROHOUR guard), changing behavior relative to the old empty-stub approach; column alignment in startingMoneyFieldParseTable also not aligned to codebase convention. |
| Generals/Code/GameEngine/Source/Common/MultiplayerSettings.cpp | Adds addStartingMoneyChoice implementation and initializes previously-uninitialized m_gotDefaultStartingMoney = false in the constructor; both changes are correct and resolve the linker error described in the PR. |
| Core/GameEngine/CMakeLists.txt | Uncomments 24 INI source file entries in the Core build, correctly mirroring the removals in Generals and GeneralsMD CMakeLists files. |
| Generals/Code/GameEngine/CMakeLists.txt | Comments out 24 INI source file entries previously compiled from the Generals tree, deferring to the new Core versions; change is symmetric with Core additions. |
| GeneralsMD/Code/GameEngine/CMakeLists.txt | Comments out the same 24 INI source entries in the Zero Hour build to use the unified Core versions instead; straightforward and correct. |
| scripts/cpp/unify_move_files.py | Adds 24 commented-out unify_file(...) entries to document the files that were migrated in this PR; purely historical record-keeping, no functional impact. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before["Before this PR"]
G_INI["Generals/…/INI/INIMultiplayer.cpp\n(parseMultiplayerStartingMoneyChoice = empty stub)"]
MD_INI["GeneralsMD/…/INI/INIMultiplayer.cpp\n(full implementation)"]
G_MP["Generals/…/MultiplayerSettings.cpp\n(NO addStartingMoneyChoice impl,\nm_gotDefaultStartingMoney uninitialised)"]
MD_MP["GeneralsMD/…/MultiplayerSettings.cpp\n(addStartingMoneyChoice impl)"]
end
subgraph After["After this PR"]
CORE_INI["Core/…/INI/INIMultiplayer.cpp ✨\n(full implementation for BOTH builds)"]
G_MP2["Generals/…/MultiplayerSettings.cpp ✨\n(addStartingMoneyChoice added,\nm_gotDefaultStartingMoney = false)"]
MD_MP2["GeneralsMD/…/MultiplayerSettings.cpp\n(unchanged)"]
end
subgraph CMake["CMakeLists changes"]
C_CMAKE["Core/CMakeLists.txt — uncommented 24 INI files"]
G_CMAKE["Generals/CMakeLists.txt — commented out 24 INI files"]
MD_CMAKE["GeneralsMD/CMakeLists.txt — commented out 24 INI files"]
end
Before --> After
CORE_INI -->|"compiled into"| G_MP2
CORE_INI -->|"compiled into"| MD_MP2
C_CMAKE -.->|"enables"| CORE_INI
G_CMAKE -.->|"disables old"| G_INI
MD_CMAKE -.->|"disables old"| MD_INI
Comments Outside Diff (1)
-
Core/GameEngine/Source/Common/INI/INIMultiplayer.cpp, line 98-109 (link)Behavior change for Generals contradicts PR description
The PR description states the follow-up fix was to "keep
MultiplayerStartingMoneyChoiceparsing Zero Hour-only (RTS_ZEROHOUR) ... to preserve Generals behavior", but the actual implementation does the opposite:parseMultiplayerStartingMoneyChoiceDefinitionis now a full, active implementation compiled into both Generals and Zero Hour builds.Previously, the Generals-specific stub (
Generals/Code/GameEngine/Source/Common/INI/INIMultiplayer.cpp) was an intentional no-op:void INI::parseMultiplayerStartingMoneyChoiceDefinition( INI* ini ) { }
Now, if Generals'
multiplayer.inicontains anyMultiplayerStartingMoneyChoiceblocks, they will be parsed, stored inm_startingMoneyList, and potentially setm_defaultStartingMoney— a runtime behavior change for Generals. Worth confirming whether Generals' data files actually contain such blocks; if they do, this is not a "no intended behavior change" situation.
Last reviewed commit: 833457b
fix(common-ini): make zh guard explicit in INIMultiplayer
4c2f0fb to
32cbb90
Compare
Additional Comments (1)
This file is newly created in this PR (2026), but the GPL license header still carries Context Used: Rule from Why: ... (source) |
|
|
With unifying we usually do two steps:
At the moment it is hard to review
|
Merge with Rebase
This change merges Common INI loader code and moves shared INI loader files to Core.
Most INI files were almost identical already, so this is primarily a mechanical unification:
GeneralsMD/Code/GameEngine/Source/Common/INI/*toCore/GameEngine/Source/Common/INI/*Generals/Code/GameEngine/Source/Common/INI/*GeneralsandGeneralsMDCMakeListsCoreCMakeListsFollow-up fix included:
MultiplayerStartingMoneyChoiceparsing Zero Hour-only (RTS_ZEROHOUR) inINIMultiplayer.cppto preserveGenerals behavior and avoid unresolved external Errors.
No intended behavior change for existing Generals paths.