Skip to content

unify(map): Merge GameLogic map headers and implementations - #3189

Open
OmarAglan wants to merge 1 commit into
TheSuperHackers:mainfrom
OmarAglan:unify/gamelogic-map-merge
Open

unify(map): Merge GameLogic map headers and implementations#3189
OmarAglan wants to merge 1 commit into
TheSuperHackers:mainfrom
OmarAglan:unify/gamelogic-map-merge

Conversation

@OmarAglan

@OmarAglan OmarAglan commented Aug 22, 2026

Copy link
Copy Markdown

Merge with Rebase

This merges the Generals and Zero Hour implementations of PolygonTrigger, SidesList, and TerrainLogic together with their matching headers.

Target conditions preserve retail compatibility while making isolated Zero Hour improvements available to non-retail Generals builds:

  • Retail-compatible Generals continues writing PolygonTriggers version 3 without layer names and keeps malformed triggers.
  • Zero Hour and non-retail Generals use version 4 with layer names and discard triggers with fewer than two points.
  • Retail-compatible Generals keeps the legacy player-data parser and 1024-team assertion.
  • Zero Hour and non-retail Generals use the existing version 2 player dictionaries and 2048-team assertion.
  • The existing crater declaration and implementation are available to Zero Hour and non-retail Generals; this pull request adds no new caller.
  • Generals gains only the existing trigger version 4 constant required by its non-retail PolygonTrigger path; MapReaderWriterInfo otherwise remains target-specific.
  • SidesList.h already matches between both games except for its product copyright line.

No files are moved in this pull request.

Testing

  • git diff --check
  • Each Generals/Zero Hour header and source pair differs only by its product copyright line
  • All target guards preserve retail-compatible Generals paths and enable isolated improvements elsewhere
  • PR contains one commit
  • Generals and Zero Hour games build
  • Generals and Zero Hour WorldBuilders build

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

Copy link
Copy Markdown

PR Summary by Qodo

Unify map: merge GameLogic map implementations

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Unify Generals and Zero Hour map sources using product-gated compilation.
• Preserve per-game serialization formats and runtime behavior (no functional changes intended).
• Bring ZH-only fields/limits into shared implementations behind RTS_ZEROHOUR guards.
Diagram

graph TD
A["Map I/O (DataChunks)"] --> B["PolygonTrigger"] --> C{"Product flags"}
A --> D["SidesList"] --> C
A --> E["TerrainLogic"] --> C
C -->|"RTS_GENERALS"| F["Generals format/limits"]
C -->|"RTS_ZEROHOUR"| G["ZH fields/features"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep separate Generals/ZH sources (no unification)
  • ➕ Zero risk of accidental cross-product behavior bleed
  • ➕ Easier to reason about per-product code paths without #if clutter
  • ➖ Ongoing duplication and drift between products
  • ➖ Harder to do future refactors/moves of map code
2. Extract shared core into common module + product-specific adapters
  • ➕ Minimizes preprocessor branching in core logic
  • ➕ Clear separation of shared vs product-specific features (e.g., layerName, crater)
  • ➖ Larger change surface area (file moves, new interfaces)
  • ➖ More build-system churn; higher short-term risk than this PR’s scope

Recommendation: Given the stated goal (behavior preservation, no moves) the current approach—unifying implementations via RTS_GENERALS/RTS_ZEROHOUR guards—is appropriate and low-disruption. A follow-up PR could reduce #if density by extracting a shared core and isolating product-specific extensions, but that would be a larger architectural step than this merge-focused change.

Files changed (6) +132 / -9

Refactor (6) +132 / -9
PolygonTrigger.cppUnify polygon trigger serialization with ZH-only fields and v4 chunk +32/-0

Unify polygon trigger serialization with ZH-only fields and v4 chunk

• Adds RTS_ZEROHOUR-gated members initialization, reads/writes the ZH layer name when triggers chunk version supports it, and deletes invalid (<2 points) triggers for ZH. Switches chunk version selection to v3 for Generals and v4 for Zero Hour while keeping one implementation.

Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp

SidesList.cppGate ZH player-name chunk versions and team-count assertion by product +20/-0

Gate ZH player-name chunk versions and team-count assertion by product

• Introduces RTS_ZEROHOUR-only constants and parsing for optional dict payloads in the players data chunk. Splits the team allocation assertion threshold (1024 for Generals, 2048 for ZH) to match product behavior.

Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp

TerrainLogic.cppAdd ZH-only crater creation and align minor comment text +52/-1

Add ZH-only crater creation and align minor comment text

• Fixes a comment typo and adds the Zero Hour-only createCraterInTerrain implementation behind RTS_ZEROHOUR, enabling unified source without changing Generals behavior.

Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp

PolygonTrigger.cppMirror unified polygon trigger logic with product-gated chunk versioning +16/-0

Mirror unified polygon trigger logic with product-gated chunk versioning

• Wraps ZH-only fields (render/selection flags, layer name IO, invalid trigger cleanup) in RTS_ZEROHOUR and selects the appropriate triggers chunk version (v3 Generals, v4 ZH) under one codepath.

GeneralsMD/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp

SidesList.cppWrap ZH-only player-name chunk versions and preserve per-product team limits +10/-0

Wrap ZH-only player-name chunk versions and preserve per-product team limits

• Scopes player-name chunk version constants and optional dict reads to RTS_ZEROHOUR builds. Applies product-specific team-count assertion thresholds to keep Generals and ZH behavior consistent within unified sources.

GeneralsMD/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp

TerrainLogic.cppEnsure crater creation stays ZH-only via RTS_ZEROHOUR guard +2/-8

Ensure crater creation stays ZH-only via RTS_ZEROHOUR guard

• Adjusts preprocessor structure so createCraterInTerrain is compiled only for Zero Hour, matching the unified-source strategy without introducing behavior in Generals builds.

GeneralsMD/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp

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

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Crater method undeclared 🐞 Bug ⚙ Maintainability
Description
Generals' TerrainLogic.cpp now defines TerrainLogic::createCraterInTerrain() under RTS_ZEROHOUR, but
the Generals TerrainLogic class declaration has no such member, so compiling Generals sources with
RTS_ZEROHOUR would fail with a missing-member error.
Code

Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp[R2873-2876]

+void TerrainLogic::createCraterInTerrain(Object *obj)
+{
+	if (obj->getGeometryInfo().getIsSmall())
+		return;
Evidence
The PR adds the createCraterInTerrain definition to the Generals source under RTS_ZEROHOUR, but
the Generals header does not declare the method at all; the Zero Hour header shows that this member
is expected to exist in ZH builds.

Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp[2869-2918]
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h[301-316]
GeneralsMD/Code/GameEngine/Include/GameLogic/TerrainLogic.h[314-316]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Generals/.../TerrainLogic.cpp` adds a definition for `TerrainLogic::createCraterInTerrain()` under `#if RTS_ZEROHOUR`, but the Generals header `Generals/.../TerrainLogic.h` does not declare this member function. If the Generals source set is ever compiled with `RTS_ZEROHOUR=1` (a plausible step given the PR’s unification direction), this will not compile.
### Issue Context
- The definition exists in the Generals source file under `#if RTS_ZEROHOUR`.
- The Generals `TerrainLogic` class declaration only exposes `flattenTerrain(...)` and does not mention `createCraterInTerrain(...)`.
- Zero Hour’s header *does* declare `createCraterInTerrain(...)`, showing the intended API surface.
### Fix Focus Areas
- Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h[306-316]
- Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp[2869-2918]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. ZH trigger APIs missing 🐞 Bug ⚙ Maintainability
Description
Generals' PolygonTrigger.cpp now references Zero Hour-only members/APIs (layer name,
selection/render flags) under RTS_ZEROHOUR, but the Generals PolygonTrigger class definition does
not declare these, so compiling Generals sources with RTS_ZEROHOUR would fail.
Code

Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[R178-181]

+#if RTS_ZEROHOUR
+		if (info->version >= K_TRIGGERS_VERSION_4) {
+			pTrig->setLayerName(layerName);
+		}
Evidence
The changed Generals PolygonTrigger.cpp contains RTS_ZEROHOUR-guarded code calling
setLayerName(...), but the Generals PolygonTrigger.h lacks that method and the related fields; the
Zero Hour header includes them, confirming they are ZH-only API surface.

Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[146-183]
Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h[64-140]
GeneralsMD/Code/GameEngine/Include/GameLogic/PolygonTrigger.h[80-130]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Generals/.../PolygonTrigger.cpp` now includes `#if RTS_ZEROHOUR` blocks that use Zero Hour-only APIs (`setLayerName/getLayerName`) and initialize/select/render state fields. However, `Generals/.../PolygonTrigger.h` does not declare these members or methods. Any attempt to build the Generals source set with `RTS_ZEROHOUR=1` (or to otherwise enable those codepaths) will fail to compile.
### Issue Context
- The PR adds `setLayerName(...)` calls and writes/reads `layerName` under `RTS_ZEROHOUR` in the Generals source file.
- The Generals header lacks `m_layerName`, `m_shouldRender`, `m_selected`, and the associated accessors.
- The Zero Hour header includes these members and methods, demonstrating the intended shape.
### Fix Focus Areas
- Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h[64-140]
- Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[146-183]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources

Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 21a5be6 ⚖️ Balanced

Results up to commit 59b40a9


🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)


Remediation recommended
1. Crater method undeclared 🐞 Bug ⚙ Maintainability
Description
Generals' TerrainLogic.cpp now defines TerrainLogic::createCraterInTerrain() under RTS_ZEROHOUR, but
the Generals TerrainLogic class declaration has no such member, so compiling Generals sources with
RTS_ZEROHOUR would fail with a missing-member error.
Code

Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp[R2873-2876]

+void TerrainLogic::createCraterInTerrain(Object *obj)
+{
+	if (obj->getGeometryInfo().getIsSmall())
+		return;
Evidence
The PR adds the createCraterInTerrain definition to the Generals source under RTS_ZEROHOUR, but
the Generals header does not declare the method at all; the Zero Hour header shows that this member
is expected to exist in ZH builds.

Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp[2869-2918]
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h[301-316]
GeneralsMD/Code/GameEngine/Include/GameLogic/TerrainLogic.h[314-316]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Generals/.../TerrainLogic.cpp` adds a definition for `TerrainLogic::createCraterInTerrain()` under `#if RTS_ZEROHOUR`, but the Generals header `Generals/.../TerrainLogic.h` does not declare this member function. If the Generals source set is ever compiled with `RTS_ZEROHOUR=1` (a plausible step given the PR’s unification direction), this will not compile.

### Issue Context
- The definition exists in the Generals source file under `#if RTS_ZEROHOUR`.
- The Generals `TerrainLogic` class declaration only exposes `flattenTerrain(...)` and does not mention `createCraterInTerrain(...)`.
- Zero Hour’s header *does* declare `createCraterInTerrain(...)`, showing the intended API surface.

### Fix Focus Areas
- Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h[306-316]
- Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp[2869-2918]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. ZH trigger APIs missing 🐞 Bug ⚙ Maintainability
Description
Generals' PolygonTrigger.cpp now references Zero Hour-only members/APIs (layer name,
selection/render flags) under RTS_ZEROHOUR, but the Generals PolygonTrigger class definition does
not declare these, so compiling Generals sources with RTS_ZEROHOUR would fail.
Code

Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[R178-181]

+#if RTS_ZEROHOUR
+		if (info->version >= K_TRIGGERS_VERSION_4) {
+			pTrig->setLayerName(layerName);
+		}
Evidence
The changed Generals PolygonTrigger.cpp contains RTS_ZEROHOUR-guarded code calling
setLayerName(...), but the Generals PolygonTrigger.h lacks that method and the related fields; the
Zero Hour header includes them, confirming they are ZH-only API surface.

Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[146-183]
Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h[64-140]
GeneralsMD/Code/GameEngine/Include/GameLogic/PolygonTrigger.h[80-130]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Generals/.../PolygonTrigger.cpp` now includes `#if RTS_ZEROHOUR` blocks that use Zero Hour-only APIs (`setLayerName/getLayerName`) and initialize/select/render state fields. However, `Generals/.../PolygonTrigger.h` does not declare these members or methods. Any attempt to build the Generals source set with `RTS_ZEROHOUR=1` (or to otherwise enable those codepaths) will fail to compile.

### Issue Context
- The PR adds `setLayerName(...)` calls and writes/reads `layerName` under `RTS_ZEROHOUR` in the Generals source file.
- The Generals header lacks `m_layerName`, `m_shouldRender`, `m_selected`, and the associated accessors.
- The Zero Hour header includes these members and methods, demonstrating the intended shape.

### Fix Focus Areas
- Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h[64-140]
- Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[146-183]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Context sources

Grey Divider

Qodo Logo

Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp Outdated
Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp Outdated
Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp Outdated
@OmarAglan
OmarAglan force-pushed the unify/gamelogic-map-merge branch from 59b40a9 to 21a5be6 Compare August 22, 2026 19:44
@OmarAglan OmarAglan changed the title unify(map): Merge GameLogic map implementations unify(map): Merge GameLogic map headers and implementations Aug 22, 2026
@Mauller

Mauller commented Aug 22, 2026

Copy link
Copy Markdown

You missed the other instances of RTS_GENERALS and RTS_ZEROHOUR

#define K_TRIGGERS_VERSION_1 1
#define K_TRIGGERS_VERSION_2 2 // Added m_isWaterArea
#define K_TRIGGERS_VERSION_3 3 // Added m_isRiver & m_riverStart
#define K_TRIGGERS_VERSION_4 4 // Added layer name.

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.

this is added so the game builds, as its required by Polygon Trigger path.

@OmarAglan

Copy link
Copy Markdown
Author

/agentic_review

@OmarAglan

Copy link
Copy Markdown
Author

You missed the other instances of RTS_GENERALS and RTS_ZEROHOUR

ok, this is bad, well im working on it, maybe im still lacking, well get them all.

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

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 21a5be6

@OmarAglan
OmarAglan force-pushed the unify/gamelogic-map-merge branch from 21a5be6 to 82d9f62 Compare August 22, 2026 19:57
@OmarAglan

Copy link
Copy Markdown
Author

You missed the other instances of RTS_GENERALS and RTS_ZEROHOUR

i think i chnaged them all, hope so.

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.

2 participants