[bobtista]

• Relates to Remove d3dx8d dependency from MinGW builds #2176

Log_DX8_ErrorCode and Non_Fatal_Log_DX8_ErrorCode use D3DXGetErrorStringA to describe a failed call, and FVFInfoClass uses D3DXGetFVFVertexSize for the vertex size. Both pull in D3DX, which MinGW does not provide.

Now a local lookup logs the error code by name and hex value. It also replaces the hand-written switch after ValidateDevice in DX8Wrapper::Draw. The static FVFInfoClass::Get_FVF_Vertex_Size computes the size from the FVF bits. The two PRE_TRANSFORM_VERTEX calls in HeightMap.cpp, which are compiled out, call it directly.

Todo:

• Build win32 and vc6

[coderabbitai]

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: b49a8aa
• Latest commit, not reviewed: 5bc6e8c

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.

Walkthrough

The changes add FVF-based vertex-size calculation and use it in two HeightMap stream setups. They also map selected DX8 error codes to symbolic names for error logging and debug draw validation.

Changes

FVF Vertex Sizing

Layer / File(s)	Summary
Calculate and use FVF vertex sizes Core/Libraries/Source/WWVegas/WW3D2/dx8fvf.h, Core/Libraries/Source/WWVegas/WW3D2/dx8fvf.cpp, Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp	FVFInfoClass::Get_FVF_Vertex_Size calculates vertex size from FVF flags. Two HeightMap stream setups use this method instead of D3DXGetFVFVertexSize. The related D3DX includes and helper are removed.

DX8 HRESULT Logging

Layer / File(s)	Summary
Map and log HRESULT names Core/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp	A helper maps selected Direct3D and HRESULT values to symbolic names and returns "Unknown" for other values. Error loggers emit the name and hexadecimal result. Debug draw validation uses the helper for ValidateDevice results.

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

Change: Refactor

Possibly related PRs

• TheSuperHackers/GeneralsGameCode#3650: Removes runtime D3DXAssembleShader calls using embedded shader tokens. Both changes remove uses of D3DX APIs.

Suggested reviewers: cryotherenegade

Merge Risk: Minimal · up to b49a8

No actionable merge-blocking defect was identified in the D3DX lookup replacements. Merge after normal build checks.

Pre-merge checks: 4

Check name	Status	Explanation
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.
Description check		The description ties directly to the changeset. It names the error-logging functions (Log_DX8_ErrorCode and Non_Fatal_Log_DX8_ErrorCode) and the FVF vertex-size change in FVFInfoClass. It also covers …
Title check		The title identifies both D3DX functions replaced by local implementations.

Full details: Description check

Explanation

The description ties directly to the changeset. It names the error-logging functions (Log_DX8_ErrorCode and Non_Fatal_Log_DX8_ErrorCode) and the FVF vertex-size change in FVFInfoClass. It also covers the ValidateDevice switch replacement in DX8Wrapper::Draw and the two PRE_TRANSFORM_VERTEX calls in HeightMap.cpp. It explains the motivation, which is removing D3DX dependencies that MinGW does not provide. The build status (Win32 and VC6) is also noted. The description is specific enough to pass.

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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

• X
• Mastodon
• Reddit
• LinkedIn

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: 7009e8f3-c045-414c-8880-1cfb98372fe0

Commits

Reviewing files that changed from the base of the PR and between 2c8b933 and b49a8aa.

Files selected for processing: 3

• Core/GameEngineDevice/Source/W3DDevice/GameClient/HeightMap.cpp
• Core/Libraries/Source/WWVegas/WW3D2/dx8fvf.cpp
• Core/Libraries/Source/WWVegas/WW3D2/dx8fvf.h

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The changes since the previous review appear safe to merge.

Summary

The PR replaces two D3DX helpers with local code.

• The renderer calculates vertex sizes from FVF bits.
• DX8 errors now show a name and their hex value.

No new actionable issues or repository-rule violations were found. No previous review threads were supplied.

_{Reviews (4) · Last reviewed commit: "refactor(dx8): Use a CASE_LABEL macro fo..." · Reviewed by Greptile}

[xezon]

Looks reasonable. I assume Get_FVF_Vertex_Size is complete, but do not know.

[xezon]

Do you want to have this merged as 3 commits or squashed? Maybe the title needs some polishing?

[bobtista]

Squash is fine, the last two commits are review fixups.