-
Notifications
You must be signed in to change notification settings - Fork 94
bugfix: Prevent unit attachments being visible through fog after exiting a Tunnel Network #1522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
bugfix: Prevent unit attachments being visible through fog after exiting a Tunnel Network #1522
Conversation
…ing a Tunnel Network
I would check if this happens for contained objects being removed from a container in general. Such as garrisoned structures, vehicles exiting helicopters etc. Then this likely wants to go into the base remove from contained function call. |
It does indeed via |
So does this not happen for affected units with attachments exiting a helicopter in the fog? |
Correct, see #29 (comment). |
@@ -106,6 +106,20 @@ void TunnelContain::removeFromContain( Object *obj, Bool exposeStealthUnits ) | |||
|
|||
owningPlayer->getTunnelSystem()->removeFromContain( obj, exposeStealthUnits ); | |||
|
|||
#if !RETAIL_COMPATIBLE_CRC | |||
if (obj->getContain()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this can be cleaned up just by calling addOrRemoveObjFromWorld(obj, TRUE);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the way I had originally fixed it before making it more explicit. 😅 I went with this approach because addOrRemoveObjFromWorld(obj, TRUE);
calls some of the same logic again, and I thought it might be better if the attachment wasn't added to the pathfinding grid (even though this admittedly adds an inconsistency).
@@ -106,6 +106,9 @@ void TunnelContain::removeFromContain( Object *obj, Bool exposeStealthUnits ) | |||
|
|||
owningPlayer->getTunnelSystem()->removeFromContain( obj, exposeStealthUnits ); | |||
|
|||
#if !RETAIL_COMPATIBLE_CRC | |||
addOrRemoveObjFromWorld(obj, TRUE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This placement looks suspicious, because there is no counterpart in the TunnelContain::addToContainList
function.
The counter part appears to be at
GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Lines 334 to 342 in 87de0a0
// Which list to physically add to needs to be overridable | |
addToContainList(rider); | |
m_playerEnteredMask = rider->getControllingPlayer()->getPlayerMask(); | |
if (isEnclosingContainerFor( rider )) | |
{ | |
addOrRemoveObjFromWorld(rider, false); | |
} |
I suggest to revisit.
Fixes #29
This change prevents unit attachments from being visible through fog after exiting a Tunnel Network. The tunnel garrison logic goes through a different pathway and does not add the occupants of open containers (e.g. Avenger turrets) to the partition manager on exit. Unfortunately, this change is not retail compatible (though maybe for the best as it would introduce a disadvantage over retail).
See the comparison below:
SHROUD_COMP.mp4
Left client: This change - the blue player cannot see the turret attachment of the green Avenger through the fog ✔️
Right client: Retail build - the green player can see the turret attachment of the blue Avenger through the fog ❌