bugfix(contain): Route Helix attachment experience - #3282
Conversation
e3e91c6 to
6be0d03
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoRoute Helix attachment experience to containing Helix
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. Detached cannons reward former Helixes
|
| if ( obj->isKindOf( KINDOF_PORTABLE_STRUCTURE ) && obj->getExperienceTracker() ) | ||
| obj->getExperienceTracker()->setExperienceSink( getObject()->getID() ); |
There was a problem hiding this comment.
1. Detached cannons reward former helixes 📎 Requirement gap ≡ Correctness
onContaining assigns the portable structure's experience sink to the Helix, but onRemoving never resets that sink. When the cannon is detached and later kills an eligible target, ExperienceTracker::addExperiencePoints continues forwarding its experience to the former container.
Agent Prompt
## Issue description
The new experience routing persists after a portable structure is removed from its Helix, causing subsequent experience to be forwarded to the former container.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[407-408]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[428-435]
## Recommended Fix
In `onRemoving`, detect portable structures with an experience tracker and clear the sink with `INVALID_ID` when it still points to the current Helix. Apply the same compatibility guard used when assigning the sink.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if ( obj->isKindOf( KINDOF_PORTABLE_STRUCTURE ) && obj->getExperienceTracker() ) | ||
| obj->getExperienceTracker()->setExperienceSink( getObject()->getID() ); |
There was a problem hiding this comment.
2. Helixes still lose attachment experience 🐞 Bug ≡ Correctness
HelixContain::onContaining assigns the experience sink, but the portable-structure branch of HelixContain::addToContain returns without reaching that callback. Initial Gattling Cannons and other configured portable payloads take this branch, so their kills continue providing no experience to the Helix.
Agent Prompt
## Issue description
Portable Helix attachments bypass `HelixContain::onContaining`, so the newly added experience-sink assignment never runs for the intended attachment creation path.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[246-283]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp[396-409]
## Recommended Fix
Move or share the experience-sink assignment with the portable-specific insertion branches in `addToContain` and `addToContainList`, ensuring it executes when the first portable structure is attached without changing Helix's specialized containment bookkeeping.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
| Filename | Overview |
|---|---|
| GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp | Adds Helix experience-sink routing in a callback bypassed by the portable attachment's normal insertion path. |
Sequence Diagram
sequenceDiagram
participant Helix as HelixContain::createPayload
participant HC as HelixContain::addToContain
participant Callback as HelixContain::onContaining
participant XP as ExperienceTracker
Helix->>HC: addToContain(portable attachment)
HC->>HC: Set m_portableStructureID
Note over HC,Callback: Portable branch does not invoke onContaining
Callback--xXP: setExperienceSink(Helix ID)
Note over XP: Attachment XP remains unrouted
Prompt To Fix All With AI
### Issue 1
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp:407-408
**XP routing callback is bypassed**
The normal Helix payload path calls `HelixContain::addToContain()`, whose portable-structure branch records the attachment directly without invoking `OpenContain::addToContain()` or `onContaining()`. As a result, this new sink assignment never runs for the Gattling Cannon, so its kills still do not grant experience to the Helix.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "bugfix(contain): Route Helix attachment ..." | Re-trigger Greptile
| if ( obj->isKindOf( KINDOF_PORTABLE_STRUCTURE ) && obj->getExperienceTracker() ) | ||
| obj->getExperienceTracker()->setExperienceSink( getObject()->getID() ); |
There was a problem hiding this comment.
XP routing callback is bypassed
The normal Helix payload path calls HelixContain::addToContain(), whose portable-structure branch records the attachment directly without invoking OpenContain::addToContain() or onContaining(). As a result, this new sink assignment never runs for the Gattling Cannon, so its kills still do not grant experience to the Helix.
Prompt To Fix With AI
This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/HelixContain.cpp
Line: 407-408
Comment:
**XP routing callback is bypassed**
The normal Helix payload path calls `HelixContain::addToContain()`, whose portable-structure branch records the attachment directly without invoking `OpenContain::addToContain()` or `onContaining()`. As a result, this new sink assignment never runs for the Gattling Cannon, so its kills still do not grant experience to the Helix.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.6be0d03 to
71706fb
Compare
Closes #87
Helixes do not gain experience from kills made by their Gattling Cannon attachment.
This change routes experience from portable Helix attachments to the Helix while they are contained.