bugfix(ini): Store every debris model name in GenericObjectCreationNugget::parseDebrisObjectNames#2597
Open
Caball009 wants to merge 1 commit into
Open
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. |
Caball009
force-pushed
the
ini_change_next_token
branch
from
ecbefac to
a716f4d
Compare
Caball009
added
Gen
Caball009
changed the title
Caball009
changed the title
Author
|
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 is a bug fix, no? If every second debris model was skipped, then not all debris objects were used in the game, right? So this is a user facing fix? Can we get list of all affected Objects and Models, so that we can review this? |
Author
Perhaps I wasn't testing it correctly, but I couldn't really see a difference. Here's the complete list of skipped debris for the default ini files (only model names, no object names): |
Caball009
changed the title
Caball009
changed the title
Caball009
added
Bug
Ok that is quite a big list.
It should be testable by putting Retail vs Patch side by side and then trigger a debris event and compare the visuals. |
Author
Yeah, that's how that works. |
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
3 participants
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.
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