Skip to content

unify(helper): Move Object helper modules to Core (Withdrawn) - #3210

Closed
Jaredl-Dev wants to merge 0 commit into
TheSuperHackers:mainfrom
Jaredl-Dev:unify/object-helpers
Closed

unify(helper): Move Object helper modules to Core (Withdrawn)#3210
Jaredl-Dev wants to merge 0 commit into
TheSuperHackers:mainfrom
Jaredl-Dev:unify/object-helpers

Conversation

@Jaredl-Dev

@Jaredl-Dev Jaredl-Dev commented Aug 26, 2026

Copy link
Copy Markdown

No description provided.

@Jaredl-Dev
Jaredl-Dev force-pushed the unify/object-helpers branch from 152559a to 61e9217 Compare August 26, 2026 04:23
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 26, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Unify object helpers in Core GameEngine

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Centralizes eight object helpers shared across Generals and Zero Hour.
• Removes duplicate product copies and rewires both builds to Core sources.
• Keeps subdual-damage implementation restricted to Zero Hour APIs.
Diagram

graph TD
  Mover["Unify Script"] --> Core["Core GameEngine"] --> Helpers["Object Helpers"] --> Generals["Generals Build"]
  Helpers --> ZeroHour["Zero Hour Build"]
  Helpers --> Guard{"RTS_ZEROHOUR"} --> ZeroHour
Loading
High-Level Assessment

Centralizing identical and compatible helper modules in Core is the appropriate approach because it removes divergent product copies while preserving product-specific behavior with a narrow compile-time guard. Keeping separate copies or forwarding wrappers would retain unnecessary duplication and build maintenance.

Files changed (20) +64 / -42

Refactor (15) +0 / -0
ObjectDefectionHelper.hCentralize defection helper declaration +0/-0

Centralize defection helper declaration

• Adds the shared ObjectDefectionHelper declaration to Core, replacing product-owned copies.

Core/GameEngine/Include/GameLogic/Module/ObjectDefectionHelper.h

ObjectHelper.hCentralize object helper base declaration +0/-0

Centralize object helper base declaration

• Adds the common ObjectHelper base-class declaration to Core for reuse by both products.

Core/GameEngine/Include/GameLogic/Module/ObjectHelper.h

ObjectRepulsorHelper.hCentralize repulsor helper declaration +0/-0

Centralize repulsor helper declaration

• Adds the shared ObjectRepulsorHelper declaration to the Core module headers.

Core/GameEngine/Include/GameLogic/Module/ObjectRepulsorHelper.h

ObjectSMCHelper.hCentralize model-condition helper declaration +0/-0

Centralize model-condition helper declaration

• Adds the shared ObjectSMCHelper declaration to Core for both product builds.

Core/GameEngine/Include/GameLogic/Module/ObjectSMCHelper.h

ObjectWeaponStatusHelper.hCentralize weapon-status helper declaration +0/-0

Centralize weapon-status helper declaration

• Adds the shared ObjectWeaponStatusHelper declaration to Core, including its final-phase update behavior.

Core/GameEngine/Include/GameLogic/Module/ObjectWeaponStatusHelper.h

StatusDamageHelper.hMove status-damage helper declaration to Core +0/-0

Move status-damage helper declaration to Core

• Relocates the Zero Hour-originated StatusDamageHelper declaration into the shared Core tree, where it can compile for either product.

Core/GameEngine/Include/GameLogic/Module/StatusDamageHelper.h

SubdualDamageHelper.hMove subdual-damage helper declaration to Core +0/-0

Move subdual-damage helper declaration to Core

• Relocates the SubdualDamageHelper declaration into Core while its implementation remains product-gated.

Core/GameEngine/Include/GameLogic/Module/SubdualDamageHelper.h

TempWeaponBonusHelper.hMove temporary weapon bonus declaration to Core +0/-0

Move temporary weapon bonus declaration to Core

• Relocates the TempWeaponBonusHelper declaration into the shared Core module tree.

Core/GameEngine/Include/GameLogic/Module/TempWeaponBonusHelper.h

ObjectDefectionHelper.cppCentralize defection helper implementation +0/-0

Centralize defection helper implementation

• Adds the shared defection timer, effects, and serialization implementation to Core.

Core/GameEngine/Source/GameLogic/Object/Helper/ObjectDefectionHelper.cpp

ObjectHelper.cppCentralize object helper base implementation +0/-0

Centralize object helper base implementation

• Adds the common helper sleep scheduling and serialization implementation to Core.

Core/GameEngine/Source/GameLogic/Object/Helper/ObjectHelper.cpp

ObjectRepulsorHelper.cppCentralize repulsor helper implementation +0/-0

Centralize repulsor helper implementation

• Adds the shared repulsor-status clearing helper implementation to Core.

Core/GameEngine/Source/GameLogic/Object/Helper/ObjectRepulsorHelper.cpp

ObjectSMCHelper.cppCentralize model-condition helper implementation +0/-0

Centralize model-condition helper implementation

• Adds the shared special model-condition clearing implementation to Core.

Core/GameEngine/Source/GameLogic/Object/Helper/ObjectSMCHelper.cpp

ObjectWeaponStatusHelper.cppCentralize weapon-status helper implementation +0/-0

Centralize weapon-status helper implementation

• Adds the shared weapon-status helper lifecycle and serialization implementation to Core.

Core/GameEngine/Source/GameLogic/Object/Helper/ObjectWeaponStatusHelper.cpp

StatusDamageHelper.cppMove status-damage helper implementation to Core +0/-0

Move status-damage helper implementation to Core

• Relocates the status-damage helper implementation from the Zero Hour tree into Core without introducing Generals call sites.

Core/GameEngine/Source/GameLogic/Object/Helper/StatusDamageHelper.cpp

TempWeaponBonusHelper.cppMove temporary weapon bonus implementation to Core +0/-0

Move temporary weapon bonus implementation to Core

• Relocates the temporary weapon bonus helper implementation into Core without adding Generals call sites.

Core/GameEngine/Source/GameLogic/Object/Helper/TempWeaponBonusHelper.cpp

Other (5) +64 / -42
CMakeLists.txtBuild unified object helpers from Core +16/-16

Build unified object helpers from Core

• Activates eight object helper header/source pairs in the Core GameEngine source list so product builds consume the centralized implementations.

Core/GameEngine/CMakeLists.txt

SubdualDamageHelper.cppGuard unified subdual implementation for Zero Hour +5/-0

Guard unified subdual implementation for Zero Hour

• Moves the subdual healing helper into Core and wraps its implementation with RTS_ZEROHOUR because Generals lacks the required BodyModuleInterface methods.

Core/GameEngine/Source/GameLogic/Object/Helper/SubdualDamageHelper.cpp

CMakeLists.txtStop compiling local Generals helper copies +10/-10

Stop compiling local Generals helper copies

• Comments out the five former Generals helper header/source entries so the build resolves them from Core.

Generals/Code/GameEngine/CMakeLists.txt

CMakeLists.txtStop compiling local Zero Hour helper copies +16/-16

Stop compiling local Zero Hour helper copies

• Comments out all eight former Zero Hour helper header/source entries in favor of the centralized Core files.

GeneralsMD/Code/GameEngine/CMakeLists.txt

unify_move_files.pyRecord object helper migration commands +17/-0

Record object helper migration commands

• Adds commented migration commands documenting which shared files were unified and which Zero Hour-only files were moved into Core.

scripts/cpp/unify_move_files.py

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 9f4e3d8 ⚖️ Balanced

Results up to commit 152559a


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@Skyaero42 Skyaero42 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ZH has three files (StatusDamageHelper, SubdualDamageHelper and TempWeaponBonusHelper) that are not in generals, but are in the Helper directory. These three should also be moved

@Jaredl-Dev
Jaredl-Dev marked this pull request as draft August 26, 2026 07:00
@Jaredl-Dev
Jaredl-Dev force-pushed the unify/object-helpers branch from 61e9217 to 9f4e3d8 Compare August 26, 2026 07:58
@Jaredl-Dev
Jaredl-Dev marked this pull request as ready for review August 26, 2026 08:20
@Jaredl-Dev

Copy link
Copy Markdown
Author

ZH has three files (StatusDamageHelper, SubdualDamageHelper and TempWeaponBonusHelper) that are not in generals, but are in the Helper directory. These three should also be moved

Corrected the PR.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9f4e3d8

@Skyaero42

Copy link
Copy Markdown

SubdualDamageHelper remains guarded by RTS_ZEROHOUR because its implementation calls subdual-damage methods absent from Generals' BodyModuleInterface

Not entirely happy with this. Once BodyModuleInterface, there is a high chance we forget about this and the RTS_ZEROHOUR guard (unnecessarily) stays. Would like a second opinion on that.

@Jaredl-Dev

Copy link
Copy Markdown
Author

SubdualDamageHelper remains guarded by RTS_ZEROHOUR because its implementation calls subdual-damage methods absent from Generals' BodyModuleInterface

Not entirely happy with this. Once BodyModuleInterface, there is a high chance we forget about this and the RTS_ZEROHOUR guard (unnecessarily) stays. Would like a second opinion on that.

Sorry, I don't get what you are saying.

@Skyaero42

Skyaero42 commented Aug 28, 2026

Copy link
Copy Markdown

Sorry, I don't get what you are saying.

Let me rephrase it into a question.

If I understand correctly (I have not yet checked the code myself), SubdualDamageHelper in ZH calls functions found in the out-of-scope BodyModuleInterface.

If BodyModuleInterface would be unified as well, would the currently guarded calls in SubdualDamageHelper still be guarded (to prevent mismatch e.g.), or could the guard be lifted, as the functions now exist?

@Jaredl-Dev

Jaredl-Dev commented Aug 28, 2026

Copy link
Copy Markdown
Author

Sorry, I don't get what you are saying.

Let me rephrase it into a question.

If I understand correctly (I have not yet checked the code myself), SubdualDamageHelper in ZH calls functions found in the out-of-scope BodyModuleInterface.

If BodyModuleInterface would be unified as well, would the currently guarded calls in SubdualDamageHelper still be guarded (to prevent mismatch e.g.), or could the guard be lifted, as the functions now exist?

I believe technically, yes. They could be lifted.

Comment thread Core/GameEngine/Source/GameLogic/Object/Helper/SubdualDamageHelper.cpp Outdated
@Jaredl-Dev
Jaredl-Dev force-pushed the unify/object-helpers branch from 9f4e3d8 to 0c7aa56 Compare August 31, 2026 20:52
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown

Greptile Summary

The PR centralizes eight Object helper modules in Core while preserving product-specific compilation for SubdualDamageHelper.

  • Moves shared helper headers and implementations from GeneralsMD into Core.
  • Removes duplicate Generals helper files and updates both variant source lists.
  • Compiles the shared INTERFACE sources within each variant target, retaining the appropriate product definitions.
  • Records the completed moves in the unification script.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness or security issues identified.

The moved sources remain compiled exactly once into each variant library, and the Zero Hour-only implementation receives its required variant definition at compilation.

Important Files Changed

Filename Overview
Core/GameEngine/CMakeLists.txt Adds all eight moved helper pairs to the shared INTERFACE source list consumed by each variant library.
Core/GameEngine/Source/GameLogic/Object/Helper/SubdualDamageHelper.cpp Preserves Zero Hour-only implementation calls behind the variant compile guard.
Generals/Code/GameEngine/CMakeLists.txt Removes duplicate helper paths while continuing to receive their implementations through Core.
GeneralsMD/Code/GameEngine/CMakeLists.txt Removes local helper paths while compiling the shared sources through the Zero Hour target.
scripts/cpp/unify_move_files.py Documents the completed helper unification operations without altering script execution.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  CoreSources[Core helper INTERFACE sources] --> G[g_gameengine]
  CoreSources --> Z[z_gameengine]
  GDef[RTS_GENERALS identity] --> G
  ZDef[RTS_ZEROHOUR identity] --> Z
  G --> Generals[Generals executable]
  Z --> ZeroHour[Zero Hour executable]
  ZDef --> Subdual[Enable SubdualDamageHelper implementation]
  Subdual --> Z
Loading

Reviews (1): Last reviewed commit: "unify(helper): Move Object helper module..." | Re-trigger Greptile

@Jaredl-Dev Jaredl-Dev closed this Aug 31, 2026
@Jaredl-Dev
Jaredl-Dev force-pushed the unify/object-helpers branch from 0c7aa56 to b8756d3 Compare August 31, 2026 21:31
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown

No reviewable files after applying ignore patterns.

@Jaredl-Dev Jaredl-Dev changed the title unify(helper): Move Object helper modules to Core unify(helper): Move Object helper modules to Core (Withdrawn) Aug 31, 2026
@Jaredl-Dev
Jaredl-Dev deleted the unify/object-helpers branch August 31, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants