Skip to content

unify(common-ini): move shared INI loaders to Core#2395

Open
OmarAglan wants to merge 3 commits intoTheSuperHackers:mainfrom
OmarAglan:unify/common-ini
Open

unify(common-ini): move shared INI loaders to Core#2395
OmarAglan wants to merge 3 commits intoTheSuperHackers:mainfrom
OmarAglan:unify/common-ini

Conversation

@OmarAglan
Copy link

@OmarAglan OmarAglan commented Mar 4, 2026

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:

  • moved 24 files from GeneralsMD/Code/GameEngine/Source/Common/INI/* to Core/GameEngine/Source/Common/INI/*
  • deleted duplicate copies from Generals/Code/GameEngine/Source/Common/INI/*
  • commented file entries in Generals and GeneralsMD CMakeLists
  • uncommented matching entries in Core CMakeLists

Follow-up fix included:

  • keep MultiplayerStartingMoneyChoice parsing Zero Hour-only (RTS_ZEROHOUR) in INIMultiplayer.cpp to preserve
    Generals behavior and avoid unresolved external Errors.

No intended behavior change for existing Generals paths.

@greptile-apps
Copy link

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR consolidates 24 previously duplicated INI loader .cpp files by moving the GeneralsMD copies into Core/GameEngine/Source/Common/INI/ and updating the three CMakeLists files accordingly. A follow-up fix implements MultiplayerSettings::addStartingMoneyChoice() in Generals/Code/GameEngine/Source/Common/MultiplayerSettings.cpp (and adds the missing m_gotDefaultStartingMoney = false constructor initializer) to satisfy the linker when the now-unified INIMultiplayer.cpp — which contains the full parseMultiplayerStartingMoneyChoiceDefinition — is compiled into the Generals build.

Key observations:

  • The CMakeLists changes (Core ↔ Generals/GeneralsMD) are symmetric and correct.
  • The addStartingMoneyChoice implementation added to Generals matches the GeneralsMD version exactly, resolving the linker error.
  • The m_gotDefaultStartingMoney = false initializer closes a pre-existing undefined-behavior gap in the Generals constructor.
  • Core/GameEngine/Source/Common/INI/INIMultiplayer.cpp is the only file with substantive logic changes; the other 23 Core INI files are byte-identical moves from GeneralsMD.
  • The GPL copyright year on the newly-created Core/INIMultiplayer.cpp header reads 2025 and should be updated to 2026.

Confidence Score: 4/5

  • PR is safe to merge after the copyright year fix is applied; the structural changes are mechanical and the linker error has been resolved.
  • The PR successfully unifies 24 duplicated INI files into a shared Core module with correct CMakeLists symmetry. The critical linker issue from earlier development has been resolved by implementing addStartingMoneyChoice in Generals. The only remaining item is a minor cosmetic fix: updating the copyright year from 2025 to 2026 in the newly created INIMultiplayer.cpp file to align with project conventions.
  • Core/GameEngine/Source/Common/INI/INIMultiplayer.cpp (copyright year update)

Last reviewed commit: 7567d7f

@greptile-apps
Copy link

greptile-apps bot commented Mar 4, 2026

Additional Comments (1)

Core/GameEngine/Source/Common/INI/INIMultiplayer.cpp, line 3
GPL header copyright year out of date

This file is newly created in this PR (2026), but the GPL license header still carries 2025 as the copyright year. Per the project's convention of keeping license headers current, this should be updated to the current year.

**	Copyright 2026 Electronic Arts Inc.

Context Used: Rule from dashboard - What: Flag newly created code comments that reference dates prior to the current year (2026).

Why: ... (source)

@OmarAglan OmarAglan requested a review from Skyaero42 March 5, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants