bugfix(savegame): Fix crashes when saving a game in headless mode

[bobtista]

Summary

• Guard UI calls (message boxes, in-game messages) with headless mode checks to prevent crashes when saving/loading without a display
• Skip visual-only snapshot blocks (TerrainVisual, TacticalView, ParticleSystem, GhostObject) when saving in headless mode
• Handle null snapshot pointers that can occur when visual subsystems aren't initialized

Testing

• Run headless replay simulation that triggers a save
• Verify save completes without crashes
• Load the headless-created save in normal mode and verify it works

Todo

• [] Replicate to Generals

[greptile-apps]

Greptile Summary

Added headless mode support for save/load operations by guarding all UI calls (TheInGameUI->message(), MessageBoxOk()) with TheGlobalData && !TheGlobalData->m_headless checks, preventing crashes when no display is available. Implemented logic to skip visual-only snapshot blocks (CHUNK_TerrainVisual, CHUNK_TacticalView, CHUNK_ParticleSystem, CHUNK_GhostObject) during save operations in headless mode, and added null pointer checks for snapshot blocks that may not be initialized in headless environments. The load path gracefully handles missing blocks by skipping them.

Key Changes:

• Guards 4 UI interaction points in save/load error/success paths
• Skips 4 visual-only chunk types when saving in headless mode
• Adds null snapshot checks in both save (line 1379-1384) and load (line 1487-1494) paths
• Previous review comments addressed null-safety concerns for TheGlobalData

Confidence Score: 4/5

• Safe to merge with minor testing caveat - the untested TODO item should be verified
• The implementation correctly guards UI calls and handles null pointers, following existing codebase patterns. Previous review thread raised valid concerns about TheGlobalData null checks (already addressed in commit 87c471f) and questioned whether GhostObject blocks contain gameplay state. The PR description shows testing checkbox for "Load the headless-created save in normal mode" is unchecked, which is the main remaining risk. Score reduced from 5 to 4 due to incomplete testing verification
• No files require special attention - all changes follow consistent patterns and previous review concerns have been addressed

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp	Guards UI calls with headless checks, skips visual-only blocks during save/load in headless mode, handles null snapshots

Sequence Diagram

sequenceDiagram
    participant Headless as Headless Replay
    participant SaveGame as GameState::saveGame()
    participant XferSave as GameState::xferSaveData()
    participant UI as UI Components
    participant Visual as Visual Subsystems
    
    Headless->>SaveGame: Trigger save in headless mode
    SaveGame->>SaveGame: Check TheGlobalData && !m_headless
    alt Headless Mode Active
        SaveGame-->>UI: Skip error message
    else Normal Mode
        SaveGame->>UI: Show error/success messages
    end
    
    SaveGame->>XferSave: xferSaveData(xfer, SNAPSHOT_SAVELOAD)
    
    loop For each snapshot block
        XferSave->>XferSave: Check if snapshot == nullptr
        alt Snapshot is null
            XferSave-->>XferSave: Skip block (headless scenario)
        end
        
        XferSave->>XferSave: Check TheGlobalData && m_headless
        alt Headless Mode + Visual Block
            Note over XferSave: Skip CHUNK_TerrainVisual,<br/>CHUNK_TacticalView,<br/>CHUNK_ParticleSystem,<br/>CHUNK_GhostObject
            XferSave-->>XferSave: Skip visual-only block
        else Save block normally
            XferSave->>Visual: Save visual data
        end
    end
    
    XferSave-->>SaveGame: Save complete
    SaveGame->>SaveGame: Check TheGlobalData && !m_headless
    alt Headless Mode
        SaveGame-->>Headless: Return SC_OK (no UI message)
    else Normal Mode
        SaveGame->>UI: Show "Game Save Complete" message
        SaveGame-->>Headless: Return SC_OK
    end

Loading

[greptile-apps]

Additional Comments (1)

1. GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp, line 1494 (link)

   logic: Loading path needs nullptr check for blockInfo->snapshot. If a save created in headless mode (missing visual blocks) is loaded in normal mode where visual subsystems exist but their snapshot pointers could be nullptr, this could crash.

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{1 file reviewed, 6 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: Check if TheGlobalData is null before dereferencing. Also, CHUNK_GhostObject being skipped may cause issues when loading in normal mode.

Suggested change

	if( TheGlobalData->m_headless &&
	(blockName.compareNoCase( "CHUNK_TerrainVisual" ) == 0 ||
	blockName.compareNoCase( "CHUNK_TacticalView" ) == 0 ||
	blockName.compareNoCase( "CHUNK_ParticleSystem" ) == 0 ||
	blockName.compareNoCase( "CHUNK_GhostObject" ) == 0) )
	// Skip visual-only blocks when saving in headless mode
	if( TheGlobalData && TheGlobalData->m_headless &&
	(blockName.compareNoCase( "CHUNK_TerrainVisual" ) == 0 ||
	blockName.compareNoCase( "CHUNK_TacticalView" ) == 0 ||
	blockName.compareNoCase( "CHUNK_ParticleSystem" ) == 0 ||
	blockName.compareNoCase( "CHUNK_GhostObject" ) == 0) )

Are ghost objects purely visual, or do they contain gameplay state that would be needed when loading the save in normal mode?

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
Line: 1387:1391

Comment:
**logic:** Check if `TheGlobalData` is null before dereferencing. Also, `CHUNK_GhostObject` being skipped may cause issues when loading in normal mode.

```suggestion
			// Skip visual-only blocks when saving in headless mode
			if( TheGlobalData && TheGlobalData->m_headless &&
				(blockName.compareNoCase( "CHUNK_TerrainVisual" ) == 0 ||
				 blockName.compareNoCase( "CHUNK_TacticalView" ) == 0 ||
				 blockName.compareNoCase( "CHUNK_ParticleSystem" ) == 0 ||
				 blockName.compareNoCase( "CHUNK_GhostObject" ) == 0) )
```

 Are ghost objects purely visual, or do they contain gameplay state that would be needed when loading the save in normal mode?

How can I resolve this? If you propose a fix, please make it concise.

[bobtista]

Ghost objects are purely visual in terms of debugging a game crash or mismatch - our CI uses headless replays to check for these. If a player wants to load a checkpoint save created in headless, in theory yes they may be missing ghost objects. We could construct them on load if needed, but I think that's out of scope for this PR.

[greptile-apps]

style: Verify the save file is backwards-compatible when visual blocks are omitted in headless saves. The load path handles missing blocks gracefully, but ensure normal-mode loads of headless-created saves work correctly (as noted in the PR TODO).

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
Line: 1488:1494

Comment:
**style:** Verify the save file is backwards-compatible when visual blocks are omitted in headless saves. The load path handles missing blocks gracefully, but ensure normal-mode loads of headless-created saves work correctly (as noted in the PR TODO).

How can I resolve this? If you propose a fix, please make it concise.

[bobtista]

Added the nullcheck that greptile requested.
As discussed in 2133, we could create no-op implementations like HeadlessGhostObjectManager, HeadlessParticleSystem, etc then set up initialization to use stubs when m_headless is true (in another PR)