Skip to content

bugfix(savegame): Rebuild shroud grid on load so structures stay visible#3005

Closed
coolswood wants to merge 1 commit into
TheSuperHackers:mainfrom
coolswood:thesuperhackers-fog-loadfix
Closed

bugfix(savegame): Rebuild shroud grid on load so structures stay visible#3005
coolswood wants to merge 1 commit into
TheSuperHackers:mainfrom
coolswood:thesuperhackers-fog-loadfix

Conversation

@coolswood

Copy link
Copy Markdown

Summary

After loading a save game, fog of war creeps over the player's own base within a few seconds: static structures become gray "previously seen" ghosts that can no longer be selected or controlled.

Root cause

PartitionManager::loadPostProcess() was an empty stub. After a save load, the per-cell shroud counters (m_currentShroud) and the pending undo-queue are restored from the file (both PartitionManager::xfer version 2 and PartitionCell::xfer), but nothing re-applies the object looks those counters represent.

When the restored undo-queue drains (~m_unlookPersistDuration frames after load), removeLooker fires at every queued position. Static structures never re-look (nothing calls Object::look()/handleShroud() after load), so their cells drop to FOGGED. The restored m_partitionLastLook and the cell counters also disagree, so any later handleShroud() would double-count.

The same empty stub is present in both the Zero Hour and Generals trees.

Fix

Fill in PartitionManager::loadPostProcess() by mirroring the established TerrainLogic::setActiveBoundary rebuild (the canonical full-shroud-rebuild pattern, already used for map boundary resize):

  1. Drain the restored undo-queue so lingering undoes do not fire after the rebuild (processEntirePendingUndoShroudRevealQueue).
  2. Snapshot explored-but-not-visible (FOGGED) cells via storeFoggedCells.
  3. Release ghost-object partition data before the cell grid is torn down.
  4. Invalidate every object's sighting state (friend_prepareForMapBoundaryAdjust) so the later unlook()/unshroud() inside handleShroud() are no-ops instead of double-undoes/double-counts.
  5. Remember permanently-revealed (CLEAR) cells, then tear the cell grid down to the all-shrouded default (reset() + init()).
  6. Restore permanently-revealed cells.
  7. Re-look every object (friend_notifyOfNewMapBoundary → each object owns exactly one live looker).
  8. Restore explored cells + ghost objects + refreshShroudForLocalPlayer().

After this the undo-queue is empty, every live object owns its looker, and static structures stay CLEAR permanently.

All APIs used (storeFoggedCells, restoreFoggedCells, ShroudStatusStoreRestore, processEntirePendingUndoShroudRevealQueue, Object::friend_prepareForMapBoundaryAdjust, Object::friend_notifyOfNewMapBoundary, GhostObjectManager methods) already exist and are unchanged.

Replication

Applied to Zero Hour first, then replicated identically for Generals, per the contribution precedence guideline. The Generals replica is byte-identical.

Verification

  • Built and tested on a cross-platform port based on this tree (macOS). Loading a previously-broken save no longer fogs the player's own base; buildings remain selectable; previously-explored-but-not-visible areas stay gray (FOGGED) rather than black (SHROUDED), confirming restoreFoggedCells preserves explored state.
  • No changes to xfer, update, serialization, or the runtime look/unlook logic — only the previously-empty loadPostProcess body.

Loading a save left the partition/shroud state inconsistent. The per-cell
shroud counters and the pending undo-queue were restored from the file,
but nothing re-applied the object looks those counters represent. When the
restored undo-queue drained (~m_unlookPersistDuration frames after load)
removeLooker fired at every queued position. Static structures never
re-look, so their cells dropped to FOGGED and the player's own base became
gray, unselectable ghosts within a few seconds of loading a save.

PartitionManager::loadPostProcess was an empty stub, so the grid was never
rebuilt after load. Fill it by mirroring the established
TerrainLogic::setActiveBoundary rebuild: drain the queue, snapshot the
explored (FOGGED) and permanently-revealed (CLEAR) cells, release ghost
partition data, invalidate every object's sighting state so the later
unlook()/unshroud() are no-ops, tear the cell grid down to the all-shrouded
default, then re-look every object so each owns exactly one live looker.
Finally restore the explored cells and ghost objects and refresh the
display/radar. After this the undo-queue is empty and static structures
stay CLEAR permanently.

Applied to Zero Hour first, then replicated for Generals.
@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fills in the previously empty PartitionManager::loadPostProcess() stub in both the Generals and GeneralsMD trees to fix a save-game regression where static structures turn into unselectable fog ghosts seconds after load.

  • The fix mirrors the well-established TerrainLogic::setActiveBoundary rebuild sequence: drain the undo-queue, snapshot FOGGED/CLEAR cells, invalidate all object sighting state, tear down and rebuild the cell grid, re-look every live object, then restore explored cells and ghost data.
  • Both trees are updated identically, consistent with the project's contribution precedence guideline.

Confidence Score: 4/5

Safe to merge; the fix correctly rebuilds the shroud grid on load using an established pattern already proven in TerrainLogic::setActiveBoundary.

The sequence of operations — drain, snapshot, tear-down, rebuild, restore — is a faithful adaptation of the setActiveBoundary pattern. The deliberate omissions (TheRadar::newMap, forceCameraAreaConstraintRecalc) are correct for the load context. No logic errors, ordering issues, or rule violations were found. A small margin is kept because this touches the core fog-of-war rebuild path and has only been validated on a macOS port rather than the full target platform matrix.

No files require special attention. Both PartitionManager.cpp files are byte-identical and correctly updated.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp Fills in the empty loadPostProcess() stub by implementing the canonical shroud-grid rebuild sequence, closely mirroring TerrainLogic::setActiveBoundary. Logic and ordering match the reference implementation; no issues found.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp Byte-identical replica of the Generals fix; same correct sequence. No issues found.

Sequence Diagram

sequenceDiagram
    participant L as loadPostProcess
    participant PM as PartitionManager
    participant GOM as GhostObjectManager
    participant OBJ as GameObjects
    L->>PM: processEntirePendingUndoShroudRevealQueue()
    L->>PM: storeFoggedCells(store TRUE)
    L->>GOM: releasePartitionData()
    L->>OBJ: friend_prepareForMapBoundaryAdjust all objects
    L->>PM: storeFoggedCells(store FALSE)
    L->>PM: reset then init
    L->>PM: restoreFoggedCells(store FALSE)
    L->>GOM: lockGhostObjects TRUE
    L->>OBJ: friend_notifyOfNewMapBoundary all objects
    L->>PM: restoreFoggedCells(store TRUE)
    L->>GOM: restorePartitionData
    L->>GOM: lockGhostObjects FALSE
    L->>PM: refreshShroudForLocalPlayer
Loading

Reviews (1): Last reviewed commit: "bugfix(savegame): Rebuild shroud grid on..." | Re-trigger Greptile

@Caball009

Copy link
Copy Markdown

What does the issue look like? Could you share a video or screenshot?

@coolswood

Copy link
Copy Markdown
Author

@Caball009 Here's a recording of the issue. One upfront note on the setup: this capture was taken with the Contra 009 mod installed, because it gave us a ready-made save with a developed base to reproduce against. I want to be transparent that it's a modded capture, but I'll explain below why the mod is not the cause here.

Why the mod can't be responsible for this bug

The fix targets PartitionManager::loadPostProcess(), which in the current tree is an empty stub:

This is compiled engine code in GameLogic/Source/GameLogic/Object/PartitionManager.cpp. A .big mod archive can only ship data — INI files, textures, audio, maps. It physically cannot modify a compiled C++ function. So the empty stub executes byte-for-byte identically on vanilla retail, on our cross-platform port, and with any mod layered on top. A mod can neither introduce nor remove this behavior.

What Contra does change are per-object VisionRange / ShroudClearingRange values in its INIs. Those only affect the radius each unit/structure reveals — i.e. how large the grayed-out region is — not whether the player's own base fogs over after a load. The fogging itself comes from the restored undo-queue draining (~m_unlookPersistDuration frames after load): removeLooker fires at positions no live object owns, static structures never re-look, and their cells drop to FOGGED. That's engine logic the mod never touches.

What to watch in the video

Load the save.
Wait a few seconds.
The player's own static structures turn into gray "previously seen" ghosts (FOGGED) and can no longer be selected or controlled, even though they're the player's buildings.
The empty stub is upstream, not port- or mod-specific

For completeness, I checked main directly — the empty loadPostProcess() is present there too (it's not something our port or the mod introduced). The regression therefore reproduces on the retail Windows build the same way; the bug is platform- and mod-independent.

proof_small.mp4

@Caball009

Copy link
Copy Markdown

Thank you for your effort, but this is a duplicate of #2892.

@coolswood coolswood closed this Jul 23, 2026
@xezon

xezon commented Jul 23, 2026

Copy link
Copy Markdown

This interesting insofar that it is an attempt to tackle the issue in a generic way that sets up PartitionManager for success after saveload. Just looks a bit complicated. Maybe this can be done in simpler ways?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants