bugfix(energy): Don't increase power production for disabled power plants on save game load

[Caball009]

When loading a save game with disabled power plants, players still get the power production from those plants. That's because the power production is increased before the Object disabled flags are xfer'd. The following code is supposed to prevent the increase in power production, but the disabled flag would have to be xfer'd before this is called. This PR fixes that.

GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp

Lines 3829 to 3835 in 920c4e6

	void Object::friend_adjustPowerForPlayer( Bool incoming )
	{
	if (isDisabled() && getTemplate()->getEnergyProduction() > 0)
	{
	// Disabledness only affects Producers, not Consumers.
	return;
	}

Callstack:

generalszh.exe!Object::friend_adjustPowerForPlayer(bool incoming)
generalszh.exe!Player::becomingTeamMember(Object * obj, bool yes)
generalszh.exe!Object::setOrRestoreTeam(Team * team, bool restoring)
generalszh.exe!Object::xfer(Xfer * xfer)

TODO:

• Replicate in Generals.

[greptile-apps]

Greptile Summary

This PR fixes a save-game bug in Zero Hour where disabled power plants incorrectly contributed to the player's power production upon load. The root cause was that setOrRestoreTeam (which triggers friend_adjustPowerForPlayer via becomingTeamMember) was called before the object's disabled flags (m_disabledMask and m_disabledTillFrame) were deserialized, causing isDisabled() to always return false at that point.

Changes:

• Moves the setOrRestoreTeam block in Object::xfer from immediately after the status-bit xfer to after both m_disabledMask and m_disabledTillFrame are xfer'd, so isDisabled() is accurate when power production is evaluated.
• Updates the guard comment to reflect the broader prerequisite: "now that we have xferred our status bits and disabled data".

Notes:

• The same bug exists in the vanilla Generals counterpart (Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp, line 3641), where setOrRestoreTeam is still called before m_disabledMask is loaded at line 3681. The PR description acknowledges this with an open TODO.

Confidence Score: 5/5

• Safe to merge — the fix is a minimal, correct reordering with no format changes or side-effects.
• The change is a surgical one-block reorder within a serialisation function. All xfer operations between the old and new positions (geometry, vision, shroud data) are pure data loads with no dependency on the team being set, so deferring the team assignment does not break anything. The fix correctly ensures both disabled-state fields are populated before power production is evaluated. No P0/P1 issues found.
• No files require special attention. The open TODO about the vanilla Generals counterpart is tracked in the PR description.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp	Moves setOrRestoreTeam call from before to after m_disabledMask and m_disabledTillFrame are xfer'd, ensuring isDisabled() returns the correct value when power production is evaluated on save load.

Sequence Diagram

sequenceDiagram
    participant X as Object::xfer (XFER_LOAD)
    participant S as Object::setOrRestoreTeam
    participant P as Player::becomingTeamMember
    participant A as Object::friend_adjustPowerForPlayer

    Note over X: xfer status bits (m_status, m_scriptStatus, m_privateStatus)
    Note over X: xfer geometry / vision / shroud data
    Note over X: ✅ xfer m_disabledMask (NEW ORDER)
    Note over X: ✅ xfer m_disabledTillFrame (NEW ORDER)
    X->>S: setOrRestoreTeam(team, restoring=true)
    S->>P: becomingTeamMember(this, yes=true)
    P->>A: friend_adjustPowerForPlayer(incoming=true)
    Note over A: isDisabled() now correctly reads<br/>the already-loaded m_disabledMask
    alt isDisabled() == true && energyProduction > 0
        A-->>P: return early (no power added ✅)
    else not disabled
        A-->>P: objectEnteringInfluence(this)
    end

Loading

_{Reviews (1): Last reviewed commit: "Updated comment." | Re-trigger Greptile}

[xezon]

Can it perhaps be moved into Object::loadPostProcess or would that be too late then?

[Caball009]

Not sure. The most immediate issue I see is that the teamID would need to be accessible in Object::loadPostProcess.

[xezon]

Team id can be grapped with TeamID teamID = m_team ? m_team->getID() : TEAM_ID_INVALID;

[Caball009]

That wouldn't work because m_team wouldn't be set yet.

[xezon]

Maybe do a simple assignment in xfer:

	if( xfer->getXferMode() == XFER_LOAD )
	{
		m_team = TheTeamFactory->findTeamByID( teamID );
		if ( m_team == nullptr )
		{
			DEBUG_CRASH(( "Object::xfer - Unable to load team" ));
			throw SC_INVALID_DATA;
		}
	}

And then the full assignment in loadPostProcess

Team *team = m_team;
m_team = nullptr;
const Bool restoring = true;
setOrRestoreTeam( team, restoring );

Not sure if worth it, but that is what what loadPostProcess is for - to do logic that goes beyond loading and writing.

Setting m_team to null looks also a bit shady. This code sucks.

[Caball009]

Probably cleaner to just store the teamID in the class and use that in loadPostProcess.

[xezon]

Yes but then we have + 4 bytes in each object. Probably better to leave as is for now.

[Caball009]

Perhaps I can add a comment above it?

// OK, now that we have xferred our status bits and disabled data, it's safe to set the team...
// TheSuperHackers @todo Refactor so that this code can move to loadPostProcess.

[xezon]

Up to you.