bugfix(render): Fix Supply Dock shadowed in replay playback

[L3-M]

• Relates to Replicate relevant GenTool fixes and improvements #850
• Fixes Supply Dock is black/shadowed in Replay playback #439

This change fixes Supply Dock from being shadowed in replay playback in Generals and Zero Hour.

[Stubbjax]

This seems like a band-aid fix without addressing the underlying issue. I'd suggest investigating the issue for active games as well (switch teams in debug mode for an easy repro), which will likely lead to a cleaner solution.

FOGGED.mp4

[Caball009]

The CI indicates that the current implementation breaks compatibility with retail, which seems unnecessary.

[L3-M]

This seems like a band-aid fix without addressing the underlying issue. I'd suggest investigating the issue for active games as well (switch teams in debug mode for an easy repro), which will likely lead to a cleaner solution.

You're right, this was meant as a fix for observer in replay playback only. I will look further into it

The CI indicates that the current implementation breaks compatibility with retail, which seems unnecessary.

Yeah, I am not sure why it broke compatibility.

[xezon]

GenTool fixes this problem by calling some modified function that refreshes the ghost object states. I would need to dig myself though because I fixed it on ASM level with no symbols.

[xezon]

I dumped some info on how it is fixed/worked around in GenTool, but it is not particularly useful. #439

[L3-M]

Updated PR title and description as it is irrelevant now.

[L3-M]

Oops I forgot to remove the old changes.

[Mauller]

Looks good and appears to work as expected.

Did not see issues when playing ingame or in a replay.

[xezon]

I currently have some reservations about this change and need to study this code more.

[Mauller]

I currently have some reservations about this change and need to study this code more.

I think i have figured out what is going on here, essentially when the game is started the local player index is assigned to the ghost object manager in gamelogic::init on line 1690

TheGhostObjectManager->setLocalPlayerIndex(ThePlayerList->getLocalPlayer()->getPlayerIndex());

What this actually does is set the ghost object manager to be related to the local player, during a replay it ends up setting the ghost object manager to the player that created the replay instead of the observer.

The replay observer is then updated in the ghost object manager so it can see all of the map on line 1772.
But the rest of the replay observer configuration does not happen untill line 2297 where the game mode is checked to see if it is a replay.

The local player is then changed to be the ReplayObserver on line 2300 but the ghost object manager is not updated for the replay observer.

Just after this local player change is where the fix needs to occur and the ghost object manager needs updating for observers player index.

if(m_gameMode == GAME_REPLAY )
{
	ThePlayerList->setLocalPlayer(ThePlayerList->findPlayerWithNameKey(TheNameKeyGenerator->nameToKey("ReplayObserver")));
	TheGhostObjectManager->setLocalPlayerIndex(ThePlayerList->getLocalPlayer()->getPlayerIndex());    <<-- needs adding here instead of within partition manager
	TheRadar->forceOn(TRUE);
	ThePartitionManager->refreshShroudForLocalPlayer();
	TheControlBar->setControlBarSchemeByPlayer( ThePlayerList->getLocalPlayer());
	DEBUG_LOG(("Start of a replay game %ls, %d",ThePlayerList->getLocalPlayer()->getPlayerDisplayName().str(), ThePlayerList->getLocalPlayer()->getPlayerIndex()));
}

[Mauller]

After checking this again, it actually needs changing. See my comment in response to xezon.

[L3-M]

I moved that as requested for the replay observer case. but when there are more than 2 players, chaos happens. you can replicate it by switching teams in debug mode. I couldn’t figure out the reason, everything looks broken.

[Mauller]

I moved that as requested for the replay observer case. but when there are more than 2 players, chaos happens. you can replicate it by switching teams in debug mode. I couldn’t figure out the reason, everything looks broken.

In that instance a similar thing needs doing in the script code that switches the players team.

The ghost manager needs updating to the current player after the team switch but also resetting to clear the current cached ghost objects.

I don't think resetting the w3d ghost object manager clears the current player index but might be worth checking for the ordering.

[Mauller]

CommandXlat.cpp line 4085 case GameMessage::MSG_META_DEMO_SWITCH_TEAMS: is what does the team switching.

It looks like they had a build option for setting it to reconfigure the ghost manager, but it should always be done when the players team is changed.

[Mauller]

okay so making sure that TheGhostObjectManager->setLocalPlayerIndex(idx); is hit within case GameMessage::MSG_META_DEMO_SWITCH_TEAMS: in CommandXlat.cpp appears to work as expected.

It takes a few seconds after team switch but the original team ghosts disappear and get updated with the new teams data.

[L3-M]

okay so making sure that TheGhostObjectManager->setLocalPlayerIndex(idx); is hit within case GameMessage::MSG_META_DEMO_SWITCH_TEAMS: in CommandXlat.cpp appears to work as expected.

Yeah I tried that before but after some investigating it looks like ghost objects carry over into the next game:

1.mp4

[Mauller]

So that looks like a reset issue, either the reset is not getting called, or is not called in the right place, for the ghost object manager or the reset code itself is not clearing ghost objects properly when players are switched.

If you play it for longer than that do they eventually disappear? because i noticed when team switching it can take a few seconds for the original teams objects to disappear.

But beyond the initial fix for blacked out buildings for the replay observer, i don't think this issue is that critical at the moment since it's not something player facing since they can't switch teams under normal circumstances.

Maybe we consider this PR for fixing the replay observer case and look into the other ghost object issues in the future?

[L3-M]

Maybe we consider this PR for fixing the replay observer case and look into the other ghost object issues in the future?

Yeah, that makes sense

[L3-M]

Rebased.

[Mauller]

Looks good

[xezon]

Change #1569 implicitly obsoletes this targeted fix.