Conversation
…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. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
xezon
added
Minor
|
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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (63)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe 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 existing labels and assertion messages change. ChangesEnum and table consistency
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The enum/table checks preserve the inspected mappings, and no concrete user-facing regression is established. The change is ready to merge with normal build checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The shared declarations and compile-time checks do not show a new security exposure or changed runtime flow in the inspected consumers. Review of all dependent consumers and builds remains incomplete. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
 |
| ST_ENEMIES = 0x0080, | ||
| ST_EVERYONE = 0x0100, | ||
|
|
||
| SOUND_TYPE_END // keep after the last named flag |
There was a problem hiding this comment.
Assuming SOUND_TYPE_END is now 0x101, I don't know what that value signifies.
There are many _END additions to binary enums in this pr.
There was a problem hiding this comment.
I understand that's useful for regular enums, but not ones with binary values.
SOUND_TYPE_END == (ST_UI | ST_EVERYONE), so what's 'end' useful for here?
There was a problem hiding this comment.
It is used in static_assert:
static_assert(1 << (ARRAY_SIZE(theSoundTypeNames) - 2) == SOUND_TYPE_END - 1, "Incorrect array size");
There was a problem hiding this comment.
Ok, that looks a bit clunky to me, but it works.
There was a problem hiding this comment.
Yes looks quite a bit unholy. I can wrap it into a helper function to simplify the call sites.
| COMMANDUSABLE_GAME = (1 << 1), // Command is usable when not in Shell | ||
| COMMANDUSABLE_OBSERVER = (1 << 2), // TheSuperHackers @feature Command is usable when observing | ||
|
|
||
| COMMAND_USABLE_IN_TYPE_END, // keep after the last named flag |
There was a problem hiding this comment.
Maybe name consistency needs looking at.
2 participants
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