Skip to content

chore: Add static asserts to verify the size of arrays - #3389

Open
xezon wants to merge 3 commits into
TheSuperHackers:mainfrom
xezon:xezon/chore-array-size-static-asserts
Open

xezon wants to merge 3 commits into
TheSuperHackers:mainfrom
xezon:xezon/chore-array-size-static-asserts

Conversation

@xezon

@xezon xezon commented Sep 29, 2026

Copy link
Copy Markdown

Merge with Rebase

This change fixes 2 arrays in wdump and adds static asserts for various C arrays that correspond to enum values or bit flags. This makes the code more resilient to mistakes from editing an enumeration without updating the related arrays.

AI Use

This change was 99% generated.

TODO

  • Add pull id to commit titles
  • Replicate in Generals

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing labels Sep 29, 2026
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 79eaa9a1-7ebe-4cf1-9efe-12fa423381e4

📥 Commits

Reviewing files that changed from the base of the PR and between d22da71 and afa134f.

📒 Files selected for processing (12)
  • Core/GameEngine/Include/GameClient/ControlBar.h
  • Core/GameEngine/Include/GameClient/Image.h
  • Core/GameEngine/Include/GameClient/MetaEvent.h
  • Core/GameEngine/Source/Common/INI/INIAudioEventInfo.cpp
  • Core/GameEngine/Source/GameClient/GUI/GameWindowManagerScript.cpp
  • Core/Libraries/Include/Lib/BaseFunctions.h
  • GeneralsMD/Code/GameEngine/Include/GameClient/Shadow.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/LocomotorSet.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/Module/AIUpdate.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/Module/StealthUpdate.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/Weapon.h
  • GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

The changes add enum end or count values and compile-time array-size checks across the core engine, GeneralsMD engine, graphics libraries, and tools. Several initialized arrays now infer their sizes, and some labels and assertion messages change.

Changes

Enum and table consistency

Layer / File(s) Summary
Core enum boundaries and name-table checks
Core/GameEngine/Include/Common/*, Core/GameEngine/Include/GameClient/*, Core/GameEngine/Source/Common/INI/INIAudioEventInfo.cpp, Core/GameEngine/Source/GameClient/GUI/GameWindowManagerScript.cpp, Core/GameEngine/Source/GameClient/GlobalLanguage.cpp, GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp
Core enums gain end or count values. Compile-time checks compare audio, command, window, image, category, language, and dictionary type-name tables with their enum boundaries.
Core runtime and network tables
Core/GameEngine/Include/Common/Debug.h, Core/GameEngine/Include/GameNetwork/GameSpy/PeerDefs.h, Core/GameEngine/Source/Common/System/*, Core/GameEngine/Source/GameClient/Input/Mouse.cpp, Core/GameEngine/Source/GameNetwork/*
Debug, radar, cursor, transfer-rule, and GameSpy arrays use inferred sizes or gain compile-time size checks.
Core graphics and library tables
Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/*, Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWaterTracks.cpp, Core/Libraries/Include/Lib/BaseFunctions.h, Core/Libraries/Source/Compression/CompressionManager.cpp, Core/Libraries/Source/WWVegas/WW3D2/*, Core/Libraries/Source/WWVegas/WWDebug/wwmemlog.cpp, Core/Tools/W3DView/Vector3RndCombo.cpp
Graphics and library tables infer their sizes or gain compile-time checks. Device-type enums gain count values. Four padding entries are removed from the memory-category table. A shared template checks enum bit-flag counts.
GeneralsMD enum boundaries and checks
GeneralsMD/Code/GameEngine/Include/GameClient/Shadow.h, GeneralsMD/Code/GameEngine/Include/GameLogic/*, GeneralsMD/Code/GameEngine/Include/GameLogic/Module/*, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/SimpleObjectIterator.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/SupplyTruckAIUpdate.cpp
Gameplay enums gain end or count values. Related name tables and procedure arrays gain compile-time checks against those boundaries.
GeneralsMD gameplay array checks
GeneralsMD/Code/GameEngine/Include/GameLogic/WeaponSet.h, GeneralsMD/Code/GameEngine/Source/Common/RTS/Handicap.cpp, GeneralsMD/Code/GameEngine/Source/GameClient/{Drawable.cpp,InGameUI.cpp}, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/{Locomotor.cpp,Object.cpp,WeaponSet.cpp}, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/*, GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp
Gameplay and UI arrays infer their sizes or gain compile-time checks against declared counts. Several assertion messages change.
GeneralsMD graphics and tool tables
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DBufferManager.cpp, GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/*, GeneralsMD/Code/Tools/GUIEdit/Source/Properties.cpp, GeneralsMD/Code/Tools/WorldBuilder/src/*, GeneralsMD/Code/Tools/wdump/chunk_d.cpp
Graphics and tool tables gain inferred sizes or compile-time checks. The W3D dump label tables add shader labels and checks against enum limits.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Suggested reviewers: bobtista

Merge Risk: ⚪ Minimal · up to afa13

The PR adds compile-time table checks without introducing the identified display issues. No actionable merge blocker is established.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to afa13

The inspected changes strengthen compile-time consistency checks without changing existing flag values, accepted configuration names, or command-usability checks. No introduced or worsened security risk was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected public-header changes do not expand configuration-controlled reachability. The markers are consumed by compile-time checks, and repository inspections found no runtime uses of the new sentinel identifiers. This conclusion does not establish behavior for external consumers.

Trust Boundaries and Controls

  • observed — The existing INI control resolves bit-string tokens only through the supplied name list and rejects unmatched names. Consequently, command-usability input cannot select the new end marker through this parser. The new assertion checks cardinality; it is not a replacement for runtime input validation.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding static assertions to verify array sizes against enum values and flags.
Description check ✅ Passed The description directly explains the array fixes and static assertions, and it matches the stated pull request objectives.
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.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

[Low risk] Adds compile-time array size verification.

The PR does not appear safe to merge until the map-serialization and blend-index defects are addressed.

Summary

The PR adds compile-time checks tying enum values to array sizes and corrects shader-name tables. Changes since the prior review also alter terrain editing, map serialization, drawable interpolation, and several gameplay paths.

  • The new blend allocation can save an invalid index when the description table fills.
  • Retail-compatible map output drops cliff flags from some widened rows.
  • An undragged border-tool click can add an unintended saved boundary.

Reviews (4) · Last reviewed commit: "chore: Add static asserts to verify the ..."

Comment thread Core/GameEngine/Include/Common/AudioEventInfo.h
Comment thread Core/GameEngine/Include/GameClient/MetaEvent.h
@xezon
xezon force-pushed the xezon/chore-array-size-static-asserts branch from be8105b to d22da71 Compare October 2, 2026 08:09
@xezon xezon added the Refactor Edits the code with insignificant behavior changes, is never user facing label Oct 2, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d7c9c38e-8eca-4a77-adc2-ec210392d331

📥 Commits

Reviewing files that changed from the base of the PR and between be8105b and d22da71.

📒 Files selected for processing (12)
  • Core/GameEngine/Include/GameClient/ControlBar.h
  • Core/GameEngine/Include/GameClient/Image.h
  • Core/GameEngine/Include/GameClient/MetaEvent.h
  • Core/GameEngine/Source/Common/INI/INIAudioEventInfo.cpp
  • Core/GameEngine/Source/GameClient/GUI/GameWindowManagerScript.cpp
  • Core/Libraries/Include/Lib/BaseFunctions.h
  • GeneralsMD/Code/GameEngine/Include/GameClient/Shadow.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/LocomotorSet.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/Module/AIUpdate.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/Module/StealthUpdate.h
  • GeneralsMD/Code/GameEngine/Include/GameLogic/Weapon.h
  • GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Comment thread Core/Libraries/Include/Lib/BaseFunctions.h
@xezon
xezon force-pushed the xezon/chore-array-size-static-asserts branch from d22da71 to afa134f Compare October 2, 2026 10:53
xezon added 3 commits October 3, 2026 22:50
…hader functions

The name tables for the primary gradient and the detail color function
were shorter than their W3DSHADER_*_MAX enumerations, so dumping a
shader that uses bump environment luminance, modulate 2x, add signed,
add signed 2x, scale 2x or mod alpha add color read past the end of
the table.
Adds static_assert's for enum indexed name, lookup and function tables,
so that editing an enum fails to compile until the associated array is
updated.

Arrays that were declared with an explicit enum count now use an unsized
bound, because an explicit bound silently zero fills missing entries.
Enums without a count get one. The waveTypeInfo table gets an explicit
zero entry for WaveTypeStationary, which it previously got through zero
fill, and the <undefined> padding of the memory category names is
replaced by the assert.
@xezon
xezon force-pushed the xezon/chore-array-size-static-asserts branch from afa134f to 7717c34 Compare October 3, 2026 20:51
@xezon

xezon commented Oct 3, 2026

Copy link
Copy Markdown
Author

Rebased

@xezon

xezon commented Oct 3, 2026

Copy link
Copy Markdown
Author

@greptileai what is your complaint?

@greptile-apps

greptile-apps Bot commented Oct 3, 2026

Copy link
Copy Markdown

The complaint was about three behavioral changes included in the commit I reviewed—not the static assertions themselves:

  1. Blend allocation: when the blend-description table reaches capacity, the new allocation path can retain an invalid index.
  2. Map serialization: some widened map rows can be written without their cliff flags, producing incorrect retail-compatible map output.
  3. WorldBuilder border tool: clicking without actually dragging can still add an unintended saved border segment.

Since you rebased, those findings may now be stale or already fixed. The current static-assert changes do not themselves demonstrate those defects; I should have called out the exact affected hunks instead of summarizing them as merge blockers.

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 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