Skip to content

refactor(ini): Clean up INI type table and improve search logic#2511

Open
Caball009 wants to merge 3 commits intoTheSuperHackers:mainfrom
Caball009:refactor_findBlockParse
Open

refactor(ini): Clean up INI type table and improve search logic#2511
Caball009 wants to merge 3 commits intoTheSuperHackers:mainfrom
Caball009:refactor_findBlockParse

Conversation

@Caball009
Copy link
Copy Markdown

This PR cleans up theTypeTable and puts it in alphabetical order. It also changes the loop logic, so that the last value doesn't need to be a sentinel value for the loop logic.

See commits for a cleaner diff.

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Refactor Edits the code with insignificant behavior changes, is never user facing labels Mar 30, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR performs two clean-up tasks on the INI type dispatch table in INI.cpp: it sorts theTypeTable entries into alphabetical order (making future additions easier to place correctly), and removes the null sentinel entry { nullptr, nullptr } in favour of using ARRAY_SIZE to bound the findBlockParse loop.

  • All 62 entries from the original table are preserved — none were added or dropped.
  • The alphabetical ordering is consistent with case-sensitive strcmp ordering (e.g. "LODPreset" correctly precedes "Language" because 'O' (79) < 'a' (97) in ASCII).
  • ARRAY_SIZE is defined in Core/Libraries/Source/WWVegas/WWLib/WWCommon.h and already available via the PreRTS.h include chain, so no new dependency is introduced.
  • The companion findFieldParse function (which searches FieldParse tables that still carry their own null sentinels throughout the codebase) is intentionally left unchanged, which is appropriate — changing those would be a separate, much larger refactor.
  • Column alignment in the table is clean and consistent with the e61e83cf alignment rule.

Confidence Score: 5/5

  • Safe to merge — purely cosmetic reordering plus a straightforward sentinel-removal refactor with no behavioural changes.
  • All 62 entries are preserved and correctly present in the new table, the loop bound is correctly computed at compile time via ARRAY_SIZE, and no logic that depends on table ordering exists (linear search). No P0/P1 findings.
  • No files require special attention.

Important Files Changed

Filename Overview
Core/GameEngine/Source/Common/INI/INI.cpp Alphabetically sorted theTypeTable, removed null sentinel entry, and updated findBlockParse to use ARRAY_SIZE-bounded loop — all changes are correct and safe.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["findBlockParse(token)"] --> B["Loop: i = 0 to ARRAY_SIZE(theTypeTable)-1"]
    B --> C{"strcmp(theTypeTable[i].token, token) == 0?"}
    C -- Yes --> D["return theTypeTable[i].parse"]
    C -- No --> E{"i < ARRAY_SIZE?"}
    E -- Yes --> B
    E -- No --> F["return nullptr"]
Loading

Reviews (1): Last reviewed commit: "Simplified loop logic." | Re-trigger Greptile

@Caball009 Caball009 changed the title Refactor(ini): Clean up INI type table and improve search logic refactor(ini): Clean up INI type table and improve search logic Mar 30, 2026
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Why is this labeled as performance? If you want to optimize lookups, perhaps make a change where the type table is accessible with a map.

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

Labels

Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants