Skip to content

fix(object): Prevent crash if Object::m_drawable is a nullptr on save load#2712

Merged
xezon merged 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_object_xfer_drawable
May 17, 2026
Merged

fix(object): Prevent crash if Object::m_drawable is a nullptr on save load#2712
xezon merged 1 commit into
TheSuperHackers:mainfrom
Caball009:fix_object_xfer_drawable

Conversation

@Caball009
Copy link
Copy Markdown

@Caball009 Caball009 commented May 14, 2026

This PR fixes a potential game crash if Object::m_drawable is a nullptr when loading a save game.

I'm not sure when this happens normally, but I observed a crash here when testing something. Considering there's a check earlier (at line 3609), it makes sense to have a check here as well.

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Stability Concerns stability of the runtime Crash This is a crash, very bad Saveload Is Saveload/Xfer related labels May 14, 2026
@Caball009 Caball009 changed the title fix(object): Avoid crash if Object drawable is nullptr on save game load fix(object): Avoid crash if Object::m_drawable is a nullptr on save game load May 14, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR adds a null check on draw before calling draw->setID(drawableID) inside Object::xfer(), preventing a crash when m_drawable is nullptr during save-game load. The fix mirrors the existing null-safe ternary already used on the line that retrieves the drawable ID, making the guard consistent.

  • Added draw && to the XFER_LOAD condition in both Generals/ and GeneralsMD/ copies of Object.cpp, so setID is only called when a drawable actually exists.
  • Minor cosmetic cleanup: removed redundant blank lines inside the if block and normalized spacing around the condition parentheses.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted null guard that prevents a confirmed crash with no functional side effects.

The fix adds a single boolean guard (draw &&) that exactly mirrors the null-safety already present one line above. When draw is null, setID is now skipped rather than dereferenced, which is the only correct behavior since there is no drawable to update. The change is applied symmetrically to both the Generals and Zero Hour copies of the file, keeping them in sync.

No files require special attention.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp Adds null-guard on draw before calling draw->setID() during save-game load to prevent crash when m_drawable is nullptr; also removes redundant blank lines.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp Identical null-guard fix applied to the Zero Hour counterpart of the same Object::xfer function.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Object::xfer(Xfer *xfer)"] --> B["draw = getDrawable()"]
    B --> C{"draw != nullptr?"}
    C -- Yes --> D["drawableID = draw->getID()"]
    C -- No --> E["drawableID = INVALID_DRAWABLE_ID"]
    D --> F["xfer->xferDrawableID(&drawableID)"]
    E --> F
    F --> G{"XFER_LOAD mode?"}
    G -- No --> H["skip setID"]
    G -- Yes --> I{"draw != nullptr?\n(NEW guard)"}
    I -- No --> J["skip setID\n(crash avoided)"]
    I -- Yes --> K["draw->setID(drawableID)"]
Loading

Reviews (2): Last reviewed commit: "fix(object): Avoid crash if Object drawa..." | Re-trigger Greptile

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp Outdated
@Caball009 Caball009 force-pushed the fix_object_xfer_drawable branch from 8bd0ddc to 1db8074 Compare May 15, 2026 17:50
Copy link
Copy Markdown

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

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

Agree as well

@xezon xezon changed the title fix(object): Avoid crash if Object::m_drawable is a nullptr on save game load fix(object): Prevent crash if Object::m_drawable is a nullptr on save load May 17, 2026
@xezon xezon merged commit 8951ad9 into TheSuperHackers:main May 17, 2026
17 checks passed
@Caball009 Caball009 deleted the fix_object_xfer_drawable branch May 17, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Crash This is a crash, very bad Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker Saveload Is Saveload/Xfer related Stability Concerns stability of the runtime ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants