Skip to content

bugfix(contain): Restore retail compatibility after changes to Object::m_containedByID#2868

Open
Caball009 wants to merge 4 commits into
TheSuperHackers:mainfrom
Caball009:restore_retail_comp_helixcontain
Open

bugfix(contain): Restore retail compatibility after changes to Object::m_containedByID#2868
Caball009 wants to merge 4 commits into
TheSuperHackers:mainfrom
Caball009:restore_retail_comp_helixcontain

Conversation

@Caball009

@Caball009 Caball009 commented Jul 12, 2026

Copy link
Copy Markdown

PR #2747 forgot to set the containedByID variable for objects that make use of the helix container code. This causes a mismatch in the following 5 replays: replays.zip

This PR adds the same code that was added to Object::onContainedBy to HelixContain::addToContain and HelixContain::addToContainList to fix the issue.

Resetting the contain variables, as is done in the third commit, is not needed to restore retail compatibility. I added that code because I think it's correct to reset the contain variables when the object is removed from its container.

TODO:

  • Test against replays again.

@Caball009 Caball009 added Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour ThisProject The issue was introduced by this project, or this task is specific to this project labels Jul 12, 2026
@Caball009 Caball009 marked this pull request as ready for review July 12, 2026 20:26
@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR restores retail-compatible containment state for Helix portable structures. The main changes are:

  • Store the container ID when portable structures enter a Helix.
  • Clear the stored container ID when they leave it.
  • Preserve retail-compatible containment IDs during object serialization.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h Declares containment setters used by the updated Helix flow.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp Synchronizes portable Helix containment IDs during insertion and removal.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp Keeps containment pointer bookkeeping and retail-compatible serialization behavior aligned.

Reviews (8): Last reviewed commit: "Tweaked older TSH comment." | Re-trigger Greptile

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp Outdated
@Caball009 Caball009 force-pushed the restore_retail_comp_helixcontain branch from 355935e to 7dfc59f Compare July 12, 2026 20:36
@Caball009 Caball009 force-pushed the restore_retail_comp_helixcontain branch from 7dfc59f to fc1d119 Compare July 12, 2026 21:21
@Caball009 Caball009 marked this pull request as draft July 12, 2026 21:23
@Caball009 Caball009 marked this pull request as ready for review July 12, 2026 23:47
void onRemovedFrom( Object *removedFrom );
Int getTransportSlotCount() const;
void friend_setContainedBy( Object *containedBy ) { m_containedBy = containedBy; }
void friend_setContainedBy( Object *containedBy, UnsignedInt frame ) { m_containedBy = containedBy; m_containedByFrame = frame; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it could change CRC, no?

@Caball009 Caball009 Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've made some changes so I'm hopefully not introducing changes to the logic without a retail compatibility macro.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely need to run it against more replays, though.

void onRemovedFrom( Object *removedFrom );
Int getTransportSlotCount() const;
void friend_setContainedBy( Object *containedBy ) { m_containedBy = containedBy; }
void friend_setContainedBy( Object *containedBy, UnsignedInt frame ) { m_containedBy = containedBy; m_containedByFrame = frame; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe do

void friend_setContainedBy( Object *containedBy )
{
  m_containedBy = containedBy;
  m_containedByFrame = m_containedBy ? TheGameLogic->getFrame() : 0;
}

This way the caller cannot set a wrong frame

@Caball009 Caball009 force-pushed the restore_retail_comp_helixcontain branch 2 times, most recently from 321c3d5 to bba092c Compare July 13, 2026 21:45
@Caball009 Caball009 force-pushed the restore_retail_comp_helixcontain branch from bba092c to 5da1074 Compare July 13, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker ThisProject The issue was introduced by this project, or this task is specific to this project ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants