refactor(w3ddevice): Read matrix rows through their components instead of Vector3 casts by CryoTheRenegade · Pull Request #3605 · TheSuperHackers/GeneralsGameCode

CryoTheRenegade · 2026-10-26T16:15:39Z

Some renderer code reads a Matrix4x4 row, a Vector4, through a Vector3 reference. Accessing an object through a reference to an unrelated type is undefined behavior, the same problem as #3521. This is a follow-up to #3532 and #3580, which fix the other casts of this kind.

The water code no longer needs the casts: it now reads the camera axes and position straight from the Matrix3D. The shadow code builds a Vector3 from each row's X, Y and Z. bit_cast can't do this because Vector3 and Vector4 aren't trivially copyable and differ in size.

Changes

WaterRenderObjClass::renderMirror and the WATER_TYPE_1_FB_REFLECTION case of WaterRenderObjClass::Render had the same 25-line block. It copied the camera transform into a Matrix4x4, transposed it, and cast each row to reflect it across the water plane. The transposed rows are just the Matrix3D columns. A new file-static reflectCameraTransform reflects Get_X_Vector, Get_Y_Vector, Get_Z_Vector and Get_Translation instead, and both places call it. The arithmetic per component is unchanged.
In Render, the reflected transform is only used inside an #if 0 block, so its declaration moved into that block.
W3DVolumetricShadow::updateMeshVolume in both games compares three rows of the previous and current transforms to decide whether the mesh is rotating. The three nested if/else checks are now one loop. A new file-static getRowXYZ returns a row's X, Y and Z.

The #ifdef CNC3 block in the Zero Hour updateMeshVolume has the same casts. That block is never compiled, so I left it alone.

AI Disclosure

Claude Opus 5.5 was used to aid in implementation.

…d of Vector3 casts The water reflection and the volumetric shadow rotation check read the X, Y and Z of Matrix4x4 rows by casting the Vector4 rows to Vector3 references. Build Vector3 values from the row components instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector · 2026-10-26T16:15:49Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-26T16:15:45.374593Z We couldn’t complete this review. Comment `@codex review` to try again.	`bdadbef`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-10-26T16:15:52Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

coderabbitai · 2026-10-26T16:16:02Z

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.

Last reviewed commit: e4cf2b6

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

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: ff01e2cf-2108-4564-bed8-ca84c5b13bde

Commits

Reviewing files that changed from the base of the PR and between dd8a4a8 and e4cf2b6.

Files selected for processing: 1

Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.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

Water reflection paths now use shared helpers to construct reflected camera transforms. Volumetric-shadow rotation checks in both game variants use extracted matrix-row vectors, with different dot-product comparisons.

Changes

Water Reflection

Layer / File(s)	Summary
Reflected camera transform `Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp`	The mirror and simple-rendering reflection paths use shared helpers to reflect camera transforms. The helpers reflect direction axes across a plane through the origin and translation across the water-level plane.

Volumetric Shadow Rotation

Layer / File(s)	Summary
Transform-row rotation checks `Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DVolumetricShadow.cpp`, `GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DVolumetricShadow.cpp`	Both variants add `getRowXYZ` and loop over three transform rows. The Generals variant compares absolute dot products with the threshold. The GeneralsMD variant compares signed dot products, so negative values trigger rotation detection.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

TheSuperHackers/GeneralsGameCode#3580: Replaces Vector3 reference casts with explicit component copies at other rendering-code call sites.

Suggested reviewers: xezon

Merge Risk: Minimal · up to e4cf2

No actionable merge-blocking issue was identified in the reviewed changes.

Pre-merge checks: 4

Check name	Status	Explanation
Description check		The description explains the undefined-behavior fix and the water and shadow refactors covered by the changeset.
Title check		The title clearly summarizes the main change: replacing Vector3 casts when reading matrix rows in the renderer.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 · 2026-10-26T16:18:26Z

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the latest scope changes preserve all uses of the reflected camera transform.

Summary

The PR replaces matrix casts with direct component reads.

Water reflections build the camera view from its matrix axes.
Volumetric shadows compare mesh turns without row casts.

_{Reviews (3) · Last reviewed commit: "refactor(w3ddevice): Declare the reflect..." · Reviewed by Greptile}

Skyaero42 · 2026-10-27T15:46:29Z

Why is this refactor any better than what the code was before? It is still an unreadable mess with long lines and repeated snippets.

…and loop the shadow row checks Both water reflection paths now call one reflectCameraTransform helper that reads the camera axes and position from the Matrix3D, instead of copying it into a transposed Matrix4x4. The volumetric shadow rotation check loops over the three rows instead of nesting three if/else blocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CryoTheRenegade · 2026-10-27T16:55:52Z

Why is this refactor any better than what the code was before? It is still an unreadable mess with long lines and repeated snippets.

Fair, reworked it a bit and updated the description

As for why it's better than before: reading a Vector4 through a Vector3& is undefined behavior (same issue as #3521), so its better to have them replaced with copies.

xezon

This is nice.

…the branch that uses it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…d of Vector3 casts (TheSuperHackers#3605) The water reflection and the volumetric shadow rotation check read the X, Y and Z of Matrix4x4 rows by casting the Vector4 rows to Vector3 references. Build Vector3 values from the row components instead. Both water reflection paths now call one reflectCameraTransform helper that reads the camera axes and position from the Matrix3D, instead of copying it into a transposed Matrix4x4. The volumetric shadow rotation check loops over the three rows instead of nesting three if/else blocks.

CryoTheRenegade mentioned this pull request Oct 26, 2026

ci(build): Cache MSVC and VC6 compilation with ccache #3608

Open

xezon approved these changes Oct 27, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp Outdated

Comment thread Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp Outdated

xezon added Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing labels Oct 27, 2026

CryoTheRenegade mentioned this pull request Oct 27, 2026

refactor(ww3d2): Replace Vector3 reference casts with explicit field copies #3580

Merged

refactor(w3ddevice): Declare the reflected water camera transform in …

e4cf2b6

…the branch that uses it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

xezon merged commit 8803fbc into TheSuperHackers:main Oct 27, 2026
28 checks passed

Navigation Menu

Repository navigation

refactor(w3ddevice): Read matrix rows through their components instead of Vector3 casts - #3605

refactor(w3ddevice): Read matrix rows through their components instead of Vector3 casts#3605
xezon merged 3 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:refactor/matrix-row-casts

CryoTheRenegade commented Oct 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Uh oh!

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

Reviews paused

Uh oh!

greptile-apps Bot commented Oct 26, 2026 •

edited

Loading

Summary

Uh oh!

Skyaero42 commented Oct 27, 2026

Uh oh!

CryoTheRenegade commented Oct 27, 2026

Uh oh!

xezon left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

CryoTheRenegade commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

AI Disclosure

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Uh oh!

coderabbitai Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

greptile-apps Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Uh oh!

Skyaero42 commented Oct 27, 2026

Uh oh!

CryoTheRenegade commented Oct 27, 2026

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CryoTheRenegade commented Oct 26, 2026 •

edited

Loading

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

greptile-apps Bot commented Oct 26, 2026 •

edited

Loading