Skip to content

fix(w3d): guard against null bold font in W3DDisplayString::setFont#2893

Open
bobtista wants to merge 3 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/fix/w3d-displaystring-bold-font-null
Open

fix(w3d): guard against null bold font in W3DDisplayString::setFont#2893
bobtista wants to merge 3 commits into
TheSuperHackers:mainfrom
bobtista:bobtista/fix/w3d-displaystring-bold-font-null

Conversation

@bobtista

Copy link
Copy Markdown

W3DDisplayString::setFont dereferences the result of TheFontLibrary->getFont() for the bold hotkey font without a null check. getFont can return null when font loading fails in headless mode where no display resources exist, and the game crashes as soon as a display string receives a font.

Now setFont only sets the hotkey renderer font when the bold font lookup succeeds, matching the null tolerance at the top of the function.

Todo:

  • Verify a game save during a headless replay no longer crashes in setFont
  • Replicate to Generals

@bobtista bobtista self-assigned this Jul 19, 2026
@bobtista bobtista added the Bug Something is not working right, typically is user facing label Jul 19, 2026
@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a null guard around the bold-font lookup in W3DDisplayString::setFont for both Generals and GeneralsMD, preventing a null-pointer crash in headless replay mode where TheFontLibrary->getFont() returns nullptr because display resources are unavailable.

  • Wraps the m_textRendererHotKey.Set_Font(...) call in an if( GameFont *boldFont = ... ) variable-declaration guard, consistent with the existing font == nullptr check at the top of the function.
  • Change is applied identically to both Generals/ and GeneralsMD/ variants to keep the codebases in sync.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted null guard with no behavioral impact in the normal (non-headless) code path.

The fix correctly eliminates an unconditional null dereference by wrapping the bold font assignment in a variable-declaration guard that matches the null-tolerance pattern already in the function. Both Generals and GeneralsMD variants are updated identically. The normal code path is unchanged: when the font library returns a valid pointer, Set_Font is called exactly as before.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp Adds null guard for bold font lookup in setFont; fix is correct and mirrors the null-tolerance pattern already present in the function.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp Identical null guard applied to the Zero Hour variant; change is in sync with the Generals copy.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["W3DDisplayString::setFont(font)"] --> B{font == nullptr?}
    B -- Yes --> C[return early]
    B -- No --> D{m_font == font?}
    D -- Yes --> E[return early, no-op]
    D -- No --> F["DisplayString::setFont(font)"]
    F --> G["m_textRenderer.Set_Font(m_font->fontData)"]
    G --> H["boldFont = TheFontLibrary->getFont(name, size, TRUE)"]
    H --> I{boldFont != nullptr?}
    I -- "Yes (normal mode)" --> J["m_textRendererHotKey.Set_Font(boldFont->fontData)"]
    I -- "No (headless/no display resources)" --> K["skip hotkey renderer — no crash"]
    J --> L["computeExtents()"]
    K --> L
    L --> M["m_fontChanged = TRUE"]
Loading

Reviews (2): Last reviewed commit: "fix(w3d): Simplify null bold font guard" | Re-trigger Greptile

Comment thread Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp Outdated
Comment thread GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp Outdated
@Skyaero42

Copy link
Copy Markdown

This is a third PR that references a closed PR. As I mentioned in of the other PR's, I highly disagree with the process first. This should have been an issue first (unable to save in headless), that should have had three subissues outlining three different problems why saving in headless was not possible.

The scope is too blurry now. Do these three PR's combined fix the problem?

@bobtista

Copy link
Copy Markdown
Author

This is a third PR that references a closed PR. As I mentioned in of the other PR's, I highly disagree with the process first. This should have been an issue first (unable to save in headless), that should have had three subissues outlining three different problems why saving in headless was not possible.

The scope is too blurry now. Do these three PR's combined fix the problem?

At the time I made the PR these were based on, there was a crash, but since then, other PRs were merged and the crash no longer happens. This is a good guard to have, but there's no issue for it to resolve - want me to just close it?

Comment thread Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp Outdated
Comment thread Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp Outdated

@xezon xezon 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.

Looking good

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

Labels

Bug Something is not working right, typically is user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants