unify(map): Merge and move GameLogic map headers and implementations to Core by OmarAglan · Pull Request #3189 · TheSuperHackers/GeneralsGameCode

OmarAglan · 2026-08-22T19:14:20Z

Merge with Rebase

Relates to Merge Generals and Zero Hour code into one #555

This merges and moves MapReaderWriterInfo.h, PolygonTrigger, SidesList, and TerrainLogic together with their matching headers.

Target conditions preserve retail compatibility while sharing isolated Zero Hour improvements that do not affect the Generals CRC path:

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.
All targets use the existing version-aware player-data parser and 2048-team assertion; version 1 player data follows the legacy parsing path.
The existing crater declaration and implementation are available to both games; this pull request adds no new caller in Generals.

Testing

git diff --check
Generals and Zero Hour games build
Generals and Zero Hour WorldBuilders build

qodo-free-for-open-source-projects · 2026-08-22T19:15:26Z

PR Summary by Qodo

Unify map: merge GameLogic map implementations

✨ Enhancement 🕐 20-40 Minutes

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"]

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 · 2026-08-22T19:19:18Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (2) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Retail parser reads v2 dictionaries 📎 Requirement gap ≡ Correctness

Description

ParsePlayersDataChunk unconditionally enables the Zero Hour version-2 dictionary layout for
retail-compatible Generals instead of retaining its legacy parser. Version-2 chunks will therefore
consume an extra integer and dictionaries on a target whose original parsing behavior must remain
unchanged.

Code

Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp[R437-440]
+	Int readDicts = 0;
+	if (info->version >= K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2) {
+		readDicts = file.readInt();
+	}

Evidence
Rules 1 and 2 require preserving each game's original target-specific functionality. The changed
Generals implementation defines version 2 and reads its additional readDicts field without the
retail-compatibility guard used elsewhere in this PR.
Unify Generals and Zero Hour in One Codebase
Build Two Distinct Game Executables
Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp[432-448]

Agent prompt

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

## Issue description
Retail-compatible Generals now uses the Zero Hour version-2 player-data parser, changing required target-specific behavior.
## Issue Context
Keep the legacy parser for `RTS_GENERALS && RETAIL_COMPATIBLE_CRC`; enable the version-2 dictionary format only for Zero Hour and non-retail Generals.
## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp[432-448]

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

2. Retail team limit changed 📎 Requirement gap ≡ Correctness

Description

The Generals team-count assertion is raised from the legacy 1024 limit to Zero Hour's 2048 limit
without a retail-compatibility guard. This changes required retail-compatible Generals behavior
rather than limiting the improvement to Zero Hour and non-retail builds.

Code

Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp[1134]
+	DEBUG_ASSERTCRASH(m_numTeams < 2048, ("%d teams have been allocated (so far). This seems excessive.", m_numTeams ));

Evidence
Rules 1 and 2 require game-specific original functionality to survive unification. The modified
Generals line now always asserts at 2048, and the file contains no RETAIL_COMPATIBLE_CRC guard to
preserve the legacy 1024 threshold.
Unify Generals and Zero Hour in One Codebase
Build Two Distinct Game Executables
Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp[1134-1134]

Agent prompt

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

## Issue description
Retail-compatible Generals no longer retains its original 1024-team assertion.
## Issue Context
Select the 1024 assertion for `RTS_GENERALS && RETAIL_COMPATIBLE_CRC` and retain the 2048 assertion for Zero Hour and non-retail Generals.
## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp[1134-1134]

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

3. 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

4. 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

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Mauller · 2026-08-22T19:49:37Z

You missed the other instances of RTS_GENERALS and RTS_ZEROHOUR

OmarAglan · 2026-08-22T19:52:15Z

/agentic_review

OmarAglan · 2026-08-22T19:53:54Z

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 · 2026-08-22T19:56:17Z

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

OmarAglan · 2026-08-22T19:59:23Z

You missed the other instances of RTS_GENERALS and RTS_ZEROHOUR

i think i chnaged them all, hope so.

stephanmeesters · 2026-08-23T23:44:53Z

You should try to minimize these guards, only add as needed. Consider when it may affect the CRC and when it doesn’t. For example deleting polygon triggers you would expect impacts CRC, but adding a function or class member or define generally doesn’t.

Mauller · 2026-08-24T07:10:48Z

You should try to minimize these guards, only add as needed. Consider when it may affect the CRC and when it doesn’t. For example deleting polygon triggers you would expect impacts CRC, but adding a function or class member or define generally doesn’t.

One of the finer nuances here that determines if it affects the CRC is how the data is handled for the CRC.

There are some instances where a whole object get's CRC'ed instead of its portion of the CRC being generated from the objects specific CRC function.
This then causes problems when new fields are introduced to the class.

But what you mentioned is true as long as the above is not a problem.

OmarAglan · 2026-08-24T08:10:47Z

You should try to minimize these guards, only add as needed. Consider when it may affect the CRC and when it doesn’t. For example deleting polygon triggers you would expect impacts CRC, but adding a function or class member or define generally doesn’t.

well, yes that make sense, will look into it, but how can i test if my chnage will impact CRC, like replays or something, a general question?
i will try to minimize the Guards as possible.

stephanmeesters · 2026-08-24T09:13:04Z

well, yes that make sense, will look into it, but how can i test if my chnage will impact CRC, like replays or something, a general question? i will try to minimize the Guards as possible.

There is no standardized vgenerals replay testing right now but you could create a new replay in a map that has polygon triggers (or add them yourself, and make sure the trigger affects the game somehow) with AI's and then replay check them with -headless -replay myreplay.rep to see if the replay mismatches after adding the new code.

OmarAglan · 2026-08-24T20:43:34Z

updated and cleaned up the merge, it now has less guards, and now the behavior is:

Retail-compatible Generals preserves malformed polygon triggers
Retail-compatible Generals writes polygon trigger version 3 without layer names
Non-retail Generals and Zero Hour remove malformed polygon triggers and write version 4 with layer names

ready for review

OmarAglan · 2026-08-24T20:43:56Z

/agentic_review

qodo-free-for-open-source-projects · 2026-08-24T20:46:04Z

Code review by qodo was updated up to the latest commit 7177417

stephanmeesters · 2026-08-25T20:58:02Z

Note that "merge with rebase" is the same as "squash merge" when you only have one commit.

Perhaps close #3190 and put the move in here? Right now it's also confusing for people to review because of the depends on

greptile-apps · 2026-08-26T18:27:10Z

Confidence Score: 5/5

The PR appears safe to merge, with no actionable new issues identified since the previous review.

Summary

This PR consolidates shared map-related headers and implementations under Core while preserving target-specific map serialization behavior.

Moves MapReaderWriterInfo, PolygonTrigger, SidesList, and TerrainLogic into the shared Core GameEngine.
Updates Generals and Zero Hour CMake source lists to consume the shared implementations.
Preserves retail-compatible Generals polygon-trigger version 3 output while retaining version 4 behavior elsewhere.
Introduces RETAIL_COMPATIBLE_DATA to distinguish file-format compatibility from CRC and save-transfer compatibility.
The only change since the previous review relocates the compatibility guidance comment to the existing retail-compatibility section.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Core[Core GameEngine map headers and implementations]
    G[Generals GameEngine]
    ZH[Zero Hour GameEngine]
    Flag{Generals and RETAIL_COMPATIBLE_DATA?}
    V3[Write PolygonTriggers version 3 without layer names]
    V4[Write PolygonTriggers version 4 with layer names]

    Core --> G
    Core --> ZH
    G --> Flag
    ZH --> V4
    Flag -->|Yes| V3
    Flag -->|No| V4

_{Reviews (15) · Last reviewed commit: "unify(map): Move GameLogic map headers a..."}

…Hackers#3189)

…heSuperHackers#3189)

OmarAglan · 2026-09-15T07:57:56Z

This change can be finalized.

what is left for this to get merged?

xezon · 2026-09-15T08:30:59Z

There are 2 open comments.

OmarAglan · 2026-09-15T09:53:34Z

There are 2 open comments.

yes the trigger points and the comment, i will add the comment, as for the trigger point i assume it can be left as it is, behind a guard, well just to be safe!

…heSuperHackers#3189)

coderabbitai · 2026-09-19T09:10:16Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8ef774bc-f3e8-427f-902c-9821ce705f0a

📥 Commits

Reviewing files that changed from the base of the PR and between b1aeb53 and ea1d6bd.

📒 Files selected for processing (19)

Core/GameEngine/CMakeLists.txt
Core/GameEngine/Include/Common/GameDefines.h
Core/GameEngine/Include/Common/MapReaderWriterInfo.h
Core/GameEngine/Include/GameLogic/PolygonTrigger.h
Core/GameEngine/Include/GameLogic/SidesList.h
Core/GameEngine/Include/GameLogic/TerrainLogic.h
Core/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp
Core/GameEngine/Source/GameLogic/Map/SidesList.cpp
Core/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
Generals/Code/GameEngine/CMakeLists.txt
Generals/Code/GameEngine/Include/Common/MapReaderWriterInfo.h
Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h
Generals/Code/GameEngine/Include/GameLogic/SidesList.h
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h
Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp
Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp
Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
GeneralsMD/Code/GameEngine/CMakeLists.txt
scripts/cpp/unify_move_files.py

💤 Files with no reviewable changes (9)

Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h
Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp
Core/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h
Generals/Code/GameEngine/Include/GameLogic/SidesList.h
Core/GameEngine/Include/GameLogic/PolygonTrigger.h
Core/GameEngine/Include/GameLogic/TerrainLogic.h
Generals/Code/GameEngine/Include/Common/MapReaderWriterInfo.h
Core/GameEngine/Include/GameLogic/SidesList.h

🚧 Files skipped from review as they are similar to previous changes (2)

Core/GameEngine/Source/GameLogic/Map/SidesList.cpp
Core/GameEngine/Include/Common/MapReaderWriterInfo.h

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The change moves map and game-logic sources into Core, removes legacy copies, adds retail compatibility guards, updates polygon-trigger serialization, and adds terrain crater generation.

Changes

Core game-engine migration

Layer / File(s)	Summary
Source migration and legacy removal `Core/GameEngine/...`, `Generals/Code/GameEngine/...`, `GeneralsMD/Code/GameEngine/CMakeLists.txt`, `scripts/cpp/unify_move_files.py`	Core build lists now include the migrated files. Legacy headers and implementations are removed or disabled. Move metadata is recorded in commented commands. Version comments and spacing are updated.
Retail compatibility and polygon serialization `Core/GameEngine/Include/Common/GameDefines.h`, `Core/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp`, `Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp`	`RETAIL_COMPATIBLE_DATA` is enabled by default. Polygon trigger reading and writing now select versions based on retail compatibility. Version 4 data includes layer names. Compatible CRC builds retain triggers with fewer than two points.
Terrain crater generation `Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp`	`createCraterInTerrain` lowers terrain inside an object radius, ignores unsupported objects, and clamps terrain height to `1`.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: merging and moving GameLogic map headers and implementations into Core.
Description check	✅ Passed	The description directly explains the file moves, shared implementation, retail compatibility behavior, and reported build testing.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: f7bab736-5b3d-4d54-8985-12751d8fe003

📥 Commits

Reviewing files that changed from the base of the PR and between 3d75bed and 35e4f35.

📒 Files selected for processing (19)

Core/GameEngine/CMakeLists.txt
Core/GameEngine/Include/Common/GameDefines.h
Core/GameEngine/Include/Common/MapReaderWriterInfo.h
Core/GameEngine/Include/GameLogic/PolygonTrigger.h
Core/GameEngine/Include/GameLogic/SidesList.h
Core/GameEngine/Include/GameLogic/TerrainLogic.h
Core/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp
Core/GameEngine/Source/GameLogic/Map/SidesList.cpp
Core/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
Generals/Code/GameEngine/CMakeLists.txt
Generals/Code/GameEngine/Include/Common/MapReaderWriterInfo.h
Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h
Generals/Code/GameEngine/Include/GameLogic/SidesList.h
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h
Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp
Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp
Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp
GeneralsMD/Code/GameEngine/CMakeLists.txt
scripts/cpp/unify_move_files.py

💤 Files with no reviewable changes (6)

Generals/Code/GameEngine/Include/GameLogic/PolygonTrigger.h
Generals/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp
Generals/Code/GameEngine/Include/Common/MapReaderWriterInfo.h
Generals/Code/GameEngine/Include/GameLogic/TerrainLogic.h
Generals/Code/GameEngine/Include/GameLogic/SidesList.h
Core/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

coderabbitai · 2026-09-19T09:16:59Z

+
+	for (Int i = iMin.x; i <= iMax.x; i++ )
+  {
+		for ( Int j=0; j <= iMax.y; j++ )


🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Start the inner loop at iMin.y.

For objects above y = 0, this loop scans rows from 0 through iMax.y. Rows below iMin.y cannot be inside the crater. The work therefore scales with the object Y coordinate instead of the crater radius.

Proposed fix

- for ( Int j=0; j <= iMax.y; j++ ) + for ( Int j=iMin.y; j <= iMax.y; j++ )

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

for ( Int j=0; j <= iMax.y; j++ )

for ( Int j=iMin.y; j <= iMax.y; j++ )

Can make a bug report for it. Outside the scope of this merge.

@xezon, acknowledged. The loop-bound issue is outside the scope of this merge.

Do you want me to open a follow-up GitHub issue for TerrainLogic::createCraterInTerrain?

You are interacting with an AI system.

pr #3258 target this i suppose?!

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

…Hackers#3189)

…heSuperHackers#3189)

…Hackers#3189)

…heSuperHackers#3189)

xezon

Makes sense.

…Hackers#3189)

…heSuperHackers#3189)

Import 18 upstream commits from TheSuperHackers up to e401710, including the weekly-2026-09-18 tag. Key upstream updates integrated: - unify(map): Move SidesList, PolygonTrigger, and TerrainLogic to Core (TheSuperHackers#3189) - bugfix(recorder): Prevent null dereference in playback without local player (TheSuperHackers#3240) - bugfix(network): Prevent LAN game room hang from long player names (TheSuperHackers#3039) - fix(gameinfo): Clear GameSlot IP on reset (TheSuperHackers#3311) - fix(ww3d2): Copy string setter inputs before freeing aliased storage (TheSuperHackers#3313) - fix(saveload): Check version instead of currentVersion in xfer (TheSuperHackers#3300, TheSuperHackers#3299) - bugfix(ingameui): Convert MessageDelayMS to logic frames (TheSuperHackers#3133) - fix(controlbar): Prevent null deref when RallyPointMarker undefined (TheSuperHackers#3304) - fix(terrainvisual): Prevent null deref in loadRequire (TheSuperHackers#3306) - fix(pointgroup): Prevent null deref when no size array given (TheSuperHackers#3280) - fix(modeldraw): Prevent null deref in handleClientRecoil (TheSuperHackers#3301) - bugfix(pathfinder): Prevent crash in processHierarchicalCell (TheSuperHackers#3296) - fix(ini): Prevent uninitialized stack variable in parseBitString8 (TheSuperHackers#3307) - refactor(particlesys): Use particle alignment enum for batching (TheSuperHackers#3309) Conflicts resolved: - Core/GameEngine/Source/GameNetwork/GameInfo.cpp: Preserved percentEncodeMapName - Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp: Preserved CopyWcharToWindowsWideChar - Generals/Code/GameEngine/Source/Common/Recorder.cpp: Preserved CRCInfo* allocation model - GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp: Preserved CRCInfo* allocation model - GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp: Aligned timeout calculation - Deleted obsolete Generals/SidesList.cpp following Core unification - Rejected upstream changes to CI/CD workflow

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

View reviewed changes

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

Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp Outdated

Mauller reviewed Aug 22, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp Outdated

Mauller reviewed Aug 22, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp Outdated

OmarAglan force-pushed the unify/gamelogic-map-merge branch from 59b40a9 to 21a5be6 Compare August 22, 2026 19:44

OmarAglan changed the title ~~unify(map): Merge GameLogic map implementations~~ unify(map): Merge GameLogic map headers and implementations Aug 22, 2026

OmarAglan commented Aug 22, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Include/Common/MapReaderWriterInfo.h Outdated

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

OmarAglan mentioned this pull request Aug 22, 2026

unify(map): Move GameLogic map headers and implementations to Core #3190

Closed

5 tasks done

OmarAglan force-pushed the unify/gamelogic-map-merge branch from 82d9f62 to 7177417 Compare August 24, 2026 20:35

qodo-free-for-open-source-projects Bot reviewed Aug 24, 2026

View reviewed changes

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

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

stephanmeesters reviewed Aug 25, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Include/Common/MapReaderWriterInfo.h

OmarAglan commented Aug 26, 2026

View reviewed changes

OmarAglan changed the title ~~unify(map): Merge GameLogic map headers and implementations~~ unify(map): Merge and move GameLogic map headers and implementations to Core Aug 26, 2026

OmarAglan force-pushed the unify/gamelogic-map-merge branch from 75e2c2c to 2ca07b0 Compare August 26, 2026 18:30

OmarAglan force-pushed the unify/gamelogic-map-merge branch from 9190d17 to d0148eb Compare September 11, 2026 10:08

OmarAglan added a commit to OmarAglan/GeneralsGameCode that referenced this pull request Sep 14, 2026

unify(map): Merge GameLogic map headers and implementations (TheSuper…

bcf2d94

…Hackers#3189)

OmarAglan added a commit to OmarAglan/GeneralsGameCode that referenced this pull request Sep 14, 2026

unify(map): Move GameLogic map headers and implementations to Core (T…

23c73bd

…heSuperHackers#3189)

OmarAglan force-pushed the unify/gamelogic-map-merge branch from d0148eb to 23c73bd Compare September 14, 2026 20:41

OmarAglan added a commit to OmarAglan/GeneralsGameCode that referenced this pull request Sep 19, 2026

unify(map): Move GameLogic map headers and implementations to Core (T…

35e4f35

…heSuperHackers#3189)

OmarAglan force-pushed the unify/gamelogic-map-merge branch from 23c73bd to 35e4f35 Compare September 19, 2026 09:10

coderabbitai Bot requested review from Mauller and xezon September 19, 2026 09:10

coderabbitai Bot reviewed Sep 19, 2026

View reviewed changes

xezon reviewed Sep 19, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameLogic/Map/SidesList.cpp

OmarAglan added a commit to OmarAglan/GeneralsGameCode that referenced this pull request Sep 19, 2026

unify(map): Merge GameLogic map headers and implementations (TheSuper…

702ceef

…Hackers#3189)

OmarAglan added a commit to OmarAglan/GeneralsGameCode that referenced this pull request Sep 19, 2026

unify(map): Move GameLogic map headers and implementations to Core (T…

b1aeb53

…heSuperHackers#3189)

OmarAglan force-pushed the unify/gamelogic-map-merge branch from 35e4f35 to b1aeb53 Compare September 19, 2026 10:40

xezon reviewed Sep 19, 2026

View reviewed changes

Comment thread Core/GameEngine/Include/Common/GameDefines.h Outdated

OmarAglan added 2 commits September 19, 2026 18:20

unify(map): Merge GameLogic map headers and implementations (TheSuper…

47dffd5

…Hackers#3189)

unify(map): Move GameLogic map headers and implementations to Core (T…

ea1d6bd

…heSuperHackers#3189)

OmarAglan force-pushed the unify/gamelogic-map-merge branch from b1aeb53 to ea1d6bd Compare September 19, 2026 15:20

xezon approved these changes Sep 19, 2026

View reviewed changes

xezon merged commit e5c486b into TheSuperHackers:main Sep 20, 2026
24 checks passed

xezon pushed a commit that referenced this pull request Sep 20, 2026

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

7a6548b

OmarAglan deleted the unify/gamelogic-map-merge branch September 20, 2026 08:05

OmarAglan mentioned this pull request Sep 20, 2026

unify(thingtemplate): Merge and move ThingTemplate to Core #3324

Merged

gamezerve pushed a commit to gamezerve/Reborn-Omega that referenced this pull request Sep 20, 2026

unify(map): Merge GameLogic map headers and implementations (TheSuper…

038b4b4

…Hackers#3189)

gamezerve pushed a commit to gamezerve/Reborn-Omega that referenced this pull request Sep 20, 2026

unify(map): Move GameLogic map headers and implementations to Core (T…

055db59

…heSuperHackers#3189)

fbraz3 mentioned this pull request Sep 20, 2026

chore(sync): merge thesuperhackers/main (release weekly-2026-09-18) fbraz3/GeneralsX#321

Merged

	for ( Int j=0; j <= iMax.y; j++ )
	for ( Int j=iMin.y; j <= iMax.y; j++ )

Conversation

OmarAglan commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

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

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mauller commented Aug 22, 2026

Uh oh!

Uh oh!

OmarAglan commented Aug 22, 2026

Uh oh!

OmarAglan commented Aug 22, 2026

Uh oh!

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

Uh oh!

OmarAglan commented Aug 22, 2026

Uh oh!

stephanmeesters commented Aug 23, 2026

Uh oh!

Mauller commented Aug 24, 2026

Uh oh!

OmarAglan commented Aug 24, 2026

Uh oh!

stephanmeesters commented Aug 24, 2026

Uh oh!

OmarAglan commented Aug 24, 2026

Uh oh!

OmarAglan commented Aug 24, 2026

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

stephanmeesters commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Uh oh!

OmarAglan commented Sep 15, 2026

Uh oh!

xezon commented Sep 15, 2026

Uh oh!

OmarAglan commented Sep 15, 2026

Uh oh!

coderabbitai Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

OmarAglan commented Aug 22, 2026 •

edited

Loading

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

edited

Loading

stephanmeesters commented Aug 25, 2026 •

edited

Loading

greptile-apps Bot commented Aug 26, 2026 •

edited

Loading

coderabbitai Bot commented Sep 19, 2026 •

edited

Loading

coderabbitai Bot Sep 19, 2026 •

edited

Loading