fix(ini): Remove superfluous call to INI::getNextTokenOrNull in GenericObjectCreationNugget::parseDebrisObjectNames#2597
Conversation
|
I mentioned this in the other PR, but i think we should just go down to having a single function External code should then check for null on the first token and gracefully handle it instead of throwing like the original variant did. |
ecbefac to
a716f4d
Compare
|
I narrowed the scope of this PR. |
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp | Wraps the superfluous in-body getNextTokenOrNull() in #if RETAIL_COMPATIBLE_CRC so non-retail builds parse all debris names correctly; change is minimal and correct. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp | Identical fix applied to the Zero Hour copy of parseDebrisObjectNames; same correctness assessment as the Generals counterpart. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["for-init: token = getNextToken()"] --> B{token != null?}
B -- No --> Z[End loop]
B -- Yes --> C["push token to m_names"]
C --> D{RETAIL_COMPATIBLE_CRC?}
D -- Yes --> E["token = getNextTokenOrNull()\n(extra advance — skips one token)"]
E --> F["for-increment: token = getNextTokenOrNull()"]
D -- No --> F
F --> B
style E fill:#f9a,stroke:#c33
style D fill:#adf,stroke:#36c
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["for-init: token = getNextToken()"] --> B{token != null?}
B -- No --> Z[End loop]
B -- Yes --> C["push token to m_names"]
C --> D{RETAIL_COMPATIBLE_CRC?}
D -- Yes --> E["token = getNextTokenOrNull()\n(extra advance — skips one token)"]
E --> F["for-increment: token = getNextTokenOrNull()"]
D -- No --> F
F --> B
style E fill:#f9a,stroke:#c33
style D fill:#adf,stroke:#36c
Reviews (1): Last reviewed commit: "Removed superfluous call to 'getNextToke..." | Re-trigger Greptile
This PR fixes a minor bug in the parsing of the ini OCL model names; every even token would be skipped.
This affects two types of OCL:
ObjectCreationList Name - CreateDebris(doesn't impact game logic AFAIK)ObjectCreationList Name - CreateObject(does impact game logic AFAIK)Example of debris with multiple model names:
https://github.com/TheSuperHackers/GeneralsGamePatch/blob/5845293dd12ab10c5f16c6f3116ce526f62dc369/Patch104pZH/GameFilesOriginalZH/Data/INI/ObjectCreationList.ini#L2130-L2138
It should have no effect on the game logic for the default ini files, but it may be different for mods that create
ObjectCreationList Name - CreateObjectwith more than one model name.GenericObjectCreationNugget::m_namesis used for the game logic here:GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp
Line 1343 in 46607a9