Skip to content

fix(ini): Add FieldParse null terminator in ArmorStore and AudioEventInfo - #3361

Open
xezon wants to merge 1 commit into
TheSuperHackers:mainfrom
xezon:xezon/fix-fieldparse-terminator
Open

xezon wants to merge 1 commit into
TheSuperHackers:mainfrom
xezon:xezon/fix-fieldparse-terminator

Conversation

@xezon

@xezon xezon commented Sep 25, 2026

Copy link
Copy Markdown

This change adds missing FieldParse null terminators in ArmorStore and AudioEventInfo.

It was found by the LLM as part of an INI related audit.

@xezon xezon added this to the Stability fixes milestone Sep 25, 2026
@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing Stability Concerns stability of the runtime Mod Relates to Mods or modding labels Sep 25, 2026
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bb261e51-c6f6-45c0-93af-b08bf96b4307

📥 Commits

Reviewing files that changed from the base of the PR and between a046a22 and 694908e.

📒 Files selected for processing (3)
  • Core/GameEngine/Source/Common/INI/INIAudioEventInfo.cpp
  • Generals/Code/GameEngine/Source/GameLogic/Object/Armor.cpp
  • GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Armor.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

The audio event information table and the armor field parse tables now end with null-entry sentinels.

Changes

Table terminators

Layer / File(s) Summary
Add null-entry terminators
Core/GameEngine/Source/Common/INI/INIAudioEventInfo.cpp, Generals/Code/GameEngine/Source/GameLogic/Object/Armor.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Armor.cpp
The audio event information table and both armor field parse tables now include null-entry terminators.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 69490

The added table terminators match the parser contract and preserve the existing named entries. No actionable merge risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 69490

The change affects 3 systems.

Changed systems: Core, Generals, GeneralsMD

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — Core (service) was modified; 1 changed file maps to changed impact.
  • observed — Generals (service) was modified; 1 changed file maps to changed impact.
  • observed — GeneralsMD (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in Core/GameEngine/Source/Common/INI/INIAudioEventInfo.cpp: Added the null-entry terminator to AudioEventInfo::m_audioEventInfo.
  • observed — Modified behavior in Generals/Code/GameEngine/Source/GameLogic/Object/Armor.cpp: The FieldParse table now ends with a null-valued sentinel after the "Armor" parser entry; the prior table contained only that entry.
  • observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Armor.cpp: The FieldParse table now includes a null-terminated sentinel after the Armor entry.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the addition of missing FieldParse null terminators in ArmorStore and AudioEventInfo.
Description check ✅ Passed The description accurately summarizes the changes and identifies the INI-related audit that found them.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds null terminators to INI parser field arrays.

The PR appears safe to merge.

Summary

Adds missing FieldParse terminators to the audio event table and both Armor tables, allowing the INI parser to stop at the end of each table.

Reviews (1) · Last reviewed commit: "fix(ini): Add FieldParse null terminator..."

@bobtista

Copy link
Copy Markdown

Good find - it looks like the same goes for TheMouseCursorFieldParseTable and TheMouseFieldParseTable in shared Mouse.cpp and TheStaticGameLODFieldParseTable and TheDynamicGameLODFieldParseTable in each game’s GameLOD.cpp.

@xezon

xezon commented Sep 25, 2026

Copy link
Copy Markdown
Author

Ah nice there is even more. Did you make a full audit? I just did the things Claude mentioned on a side quest.

@bobtista

Copy link
Copy Markdown

Ah nice there is even more. Did you make a full audit? I just did the things Claude mentioned on a side quest.

Yeah - 473 tables, 9 without a terminator. This PR fixes 3 of them, and the other 6 I listed above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Minor Severity: Minor < Major < Critical < Blocker Mod Relates to Mods or modding Stability Concerns stability of the runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants