[PedroTiago23]

I encountered some linker errors involving s_forceCleanCells and s_useFixedPathfinding while performing a clean build on Linux (CachyOS). Upon investigating the cause, I found that adding the "PreRTS.h" header to certain files resolved the issue for my build. These files are:

• PathNode.cpp

• PathfindCellInfo.cpp

• PathfindCellList.cpp

• PathfindConstants.cpp

• PathfindZoneManager.cpp

• ZoneBlock.cpp

After making this addition, the compilation completed successfully.

As I am new to contributing and still find debugging build processes challenging, I used AI to assist in investigating the issue. One of the most insightful pieces of information the AI ​​provided was a reference to PR #3429. I suspect the refactoring in that PR might be related to the issue, though this is just a hypothesis; I haven't investigated it deeply yet, as I am still getting familiar with the project.

For testing purposes, I invite everyone to try compiling the current project on a Linux machine. If you encounter compilation issues, please try using my fork by following the steps below:

git clone https://github.com/TheSuperHackers/GeneralsGameCode.git
cd GeneralsGameCode
git fetch https://github.com/PedroTiago23/GeneralsGameCode.git fix/pathfinder-prertsh-include
git switch --detach FETCH_HEAD
./scripts/docker-build.sh --clean --game zh

Assuming, of course, that Docker is installed and running.

[coderabbitai]

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 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: 34920684-86de-42b5-af40-147b14be89ab

📥 Commits

Reviewing files that changed from the base of the PR and between bc7696b and ede3078dad7ec8f0e350070ea57f319290d366c8.

📒 Files selected for processing: 8

• Core/GameEngine/Include/GameLogic/Pathfinder/PathNode.h
• Core/GameEngine/Include/GameLogic/Pathfinder/PathfindCell.h
• Core/GameEngine/Include/GameLogic/Pathfinder/PathfindCellInfo.h
• Core/GameEngine/Include/GameLogic/Pathfinder/PathfindLayer.h
• Core/GameEngine/Source/GameLogic/AI/Pathfinder/PathfindCellInfo.cpp
• Core/GameEngine/Source/GameLogic/AI/Pathfinder/PathfindConstants.cpp
• Core/Libraries/Include/Lib/Coord2D.h
• Core/Libraries/Include/Lib/ICoord2D.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Coordinate and pathfinding headers and source files add includes for math, game definitions and types, coordinates, debugging, and game memory. No declarations or runtime behavior changed.

Changes

Coordinate and pathfinding includes

Layer / File(s)	Summary
Add direct includes Core/Libraries/Include/Lib/Coord2D.h, Core/Libraries/Include/Lib/ICoord2D.h, Core/GameEngine/Include/GameLogic/Pathfinder/*, Core/GameEngine/Source/GameLogic/AI/Pathfinder/PathfindCellInfo.cpp, Core/GameEngine/Source/GameLogic/AI/Pathfinder/PathfindConstants.cpp	Coordinate headers include <math.h>. Pathfinder headers add game, coordinate, and region includes. Pathfinder source files add debug, game-memory, and game-definition includes.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: skyaero42

Merge Risk: ⚪ Minimal · up to ede30

No actionable merge-blocking risk is established by the reviewed changes.

Pre-merge checks: 4

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description explains the pathfinder header changes, the Linux linker errors, and the clean-build result. It is directly related to the pull request.
Title check		The title clearly identifies the pathfinder build change and the addition of PreRTS.h to source files. It is concise and specific.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR adds missing headers:

• Pathfinder source files now include the engine's shared header first.
• Integer coordinate headers now include trig declarations directly.

No actionable issues were found. A clean build was not run.

_{Reviews (4) · Last reviewed commit: "fix(pathfinder): correct header dependen..." · Reviewed by Greptile}

[Skyaero42]

Without PreRTS, it compiles fine with VC6 and MSCV, so maybe it is a compiler specific issue. Which compiler are you using?

PreRTS.h includes a lot of headers. I think it would be good to figure out which header(s) in particular the compiler is missing.

[PedroTiago23]

Without PreRTS, it compiles fine with VC6 and MSCV, so maybe it is a compiler-specific issue. Which compiler are you using?

PreRTS.h includes a lot of headers. I think it would be good to figure out which header(s) in particular the compiler is missing.

It's strange because I'm using VC6 inside the Docker build container. Wouldn't this mean that the environment should be the same for everyone, provided they have the project and run a clean build using the Linux Docker environment? Or am I mistaken?

In any case, here is some information that may be relevant to my development environment:

OS: Linux cachyos-x8664 7.2.9-2-cachyos x86_64 GNU/Linux

Docker version: Docker client=29.9.0; server=29.9.0

Build image used:

Image ID=sha256:4491d49efdeaa5b5c7e9896efd0ad7c08d6f8bcd4a09a3ce3ecb75677c99a8cc
Created=2026-10-14T02:15:31.858607962-03:00
Architecture=amd64

Compiler configuration:

CMAKE_BUILD_TYPE:STRING=Release
CMAKE_CXX_COMPILER:STRING=Z:/build/tools/vs6/vc98/bin/cl.exe
CMAKE_C_COMPILER:STRING=Z:/build/tools/vs6/vc98/bin/cl.exe
CMAKE_MAKE_PROGRAM:UNINITIALIZED=Z:/build/tools/ninja.exe

The build configuration also reports the following:

CMAKE_BUILD_TYPE="Release"
CMAKE_EXPORT_COMPILE_COMMANDS="ON"
CMAKE_MSVC_DEBUG_INFORMATION_FORMAT=""
CMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>DLL"
RTS_FLAGS="/W3"

-- The C compiler identification is MSVC 12.0.8804
-- The CXX compiler identification is MSVC 12.0.8804
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: Z:/build/tools/vs6/vc98/bin/cl.exe

These details also indicate that I am using the legacy VC6 compiler.

Dockerfile:

1:  FROM debian:12.10-slim
4:  ARG CMAKE_VERSION="3.31.6"
5:  ARG GIT_VERSION="2.49.0"
32: RUN wget https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-windows-x86_64.zip \
33:     && unzip cmake-${CMAKE_VERSION}-windows-x86_64.zip -d /build/tools/ \
34:     && mv /build/tools/cmake-${CMAKE_VERSION}-windows-x86_64 /build/tools/cmake \
38: RUN wget https://github.com/git-for-windows/git/releases/download/v${GIT_VERSION}.windows.1/MinGit-${GIT_VERSION}-64-bit.zip \
39:     && unzip MinGit-${GIT_VERSION}.windows.1/MinGit-${GIT_VERSION}-64-bit.zip -d /build/tools/ \
44: RUN wget https://github.com/itsmattkc/MSVC600/archive/refs/heads/master.zip \
46:     && mv /build/tools/MSVC600-master/ /build/tools/vs6 \

Regarding PreRTS.h, I agree that it includes a large number of headers. To avoid including numerous unnecessary headers in the six files, I removed PreRTS.h from all of them and investigated each file's dependencies separately by examining the compilation logs.

For reference, here is a short excerpt from the errors produced when I ran ./scripts/docker-build.sh --clean --game zh without PreRTS.h in the specified files:

PathfindCellInfo.h(49) : error C2501: 'ICoord2D' : missing storage-class or type specifiers

PathfindCellInfo.h(51) : error C2501: 'ObjectID' : missing storage-class or type specifiers

PathfindCellInfo.cpp(45) : error C2065: 'MSGNEW' : undeclared identifier

PathfindCellInfo.cpp(66) : error C2065: 'DEBUG_ASSERTCRASH' : undeclared identifier

As shown above, ICoord2D and ObjectID were reported as undeclared, while MSGNEW and DEBUG_ASSERTCRASH were unavailable.

After investigating the files in the Pathfinder directory, I identified missing types and macros in files such as PathfindCellInfo.cpp and PathfindCellList.cpp.

My approach was to add only the specific dependencies missing from each file. One clear example was the missing <math.h> include in ICoord2D.h, which uses sqrt directly. Without that include, the compiler reported:

ICoord2D.h(38) : error C2065: 'sqrt' : undeclared identifier

After making these changes and rerunning ./scripts/docker-build.sh --clean --game zh, the project compiled successfully, with no apparent issues.

However, if the project builds normally on everyone else's machine and these problems occur only in my environment, I may be overlooking something simple. I intend to try compiling the project on another machine I own, which runs Linux Mint. If a clean build fails there as well, that might indicate that this is an issue worth investigating.

Could a build cache or precompiled headers be masking the problem in other environments? I noticed that the project uses several precompiled headers, including PreRTS.h, but I have not studied the build system in sufficient depth to draw any conclusions yet.

[xezon]

Add PreRTS.h to the top of GameEngine CPP's

[PedroTiago23]

Add PreRTS.h to the top of GameEngine CPP's

Just to confirm, would you prefer me to restore #include "PreRTS.h" as the first include in the six affected .cpp files?

I initially tried replacing it with specific headers to avoid unnecessary dependencies, but I noticed the comment in GameEngine.cpp stating that PreRTS.h must go first in every GameEngine .cpp file. I understand this may be an established project convention, so I want to make sure I follow the intended approach before updating the PR.

[Mauller]

I think there is likely an issue within the docker build

[PedroTiago23]

I think there is likely an issue within the docker build

I’m using ./scripts/docker-build.sh --clean --game zh. The other script, dockerbuild.sh, identifies itself as legacy and recommends using docker-build.sh instead.

I tested a clean build of upstream/main, without my PR changes, and encountered the same compilation errors involving missing declarations such as ICoord2D and ObjectID, as well as the missing macros MSGNEW and DEBUG_ASSERTCRASH. I also rebuilt the Docker image using --no-cache --pull, but the same errors persisted.

The container invokes Z:/build/tools/vs6/vc98/bin/cl.exe, and CMake identifies the compiler as MSVC 12.0.8804.

Could you clarify which build command and configuration you use? I’d like to make sure I’m following the intended build procedure and understand whether there might be a difference between our environments.

[Skyaero42]

Add PreRTS.h to the top of GameEngine CPP's

Just to confirm, would you prefer me to restore #include "PreRTS.h" as the first include in the six affected .cpp files?

Could you at least try to see if that would solve the problem.

I am with Mauller that if it compiles on VC6 and MSVC, than it is weird it doesn't in linux/docker and would suggest a build pipeline issue. But lets see what works and doesn't work.

Btw, is this related to #3555 ?

[xezon]

Yes #include "PreRTS.h" is meant to go atop of every GameEngine cpp. If missing, problems are expected.

[Mauller]

Yes #include "PreRTS.h" is meant to go atop of every GameEngine cpp. If missing, problems are expected.

The problem might actually be related to the pre-compiled header issue for docker builds #3555

We might need it, but If the issue was not including PreRTS.h in the cpp files then it would have failed to compile in VC6 and VS22 when @Skyaero42 split the files out.

[tintinhamans]

tested this rebased onto current main. needs #include "trig.h" in ICoord2D.h now that it uses Sqrt instead of sqrt. with that added, a clean Docker/VC6 build of z_generals passes without #3555's workaround. could you rebase and add that include?

[PedroTiago23]

Thanks for the feedback! I've rebased the PR onto the current main, added trig.h to ICoord2D.h, and restored PreRTS.h as the first include in the six affected Pathfinder source files. I also removed the additional includes from the four Pathfinder headers after confirming that a clean Docker/VC6 build passes without them. The clean build now succeeds with the remaining changes.

[Skyaero42]

I had claude have a go at it

---

Why PathfindCellInfo fails in the Docker build but compiles on Windows

Cause: The Docker build turns precompiled headers off (-DCMAKE_DISABLE_PRECOMPILE_HEADERS=1 in resources/dockerbuild/entrypoint.sh). On Windows, corei_gameengine_private force-includes PreRTS.h into every GameEngine .cpp as a precompiled header. That hides missing includes. Without it, each file has to include what it uses.

PathfindCellInfo.h has no includes, and PathfindCellInfo.cpp doesn't include PreRTS.h first. That explains every error:

Error	Symbol	Defined in
C2501	ICoord2D	Lib/BaseType.h
C2501	ObjectID	Common/GameType.h
C2065	MSGNEW	Common/GameMemory.h
C2065	DEBUG_ASSERTCRASH	Common/Debug.h

The compiler isn't the difference: the Docker image uses VC6 too. VC6's cl.exe reports itself as version 12.00, hence "MSVC 12".

Fix:

1. Start PathfindCellInfo.cpp with #include "PreRTS.h", like every other GameEngine source file.
2. Make PathfindCellInfo.h self-contained:

   #pragma once
   
   #include "Lib/BaseType.h"
   #include "Common/GameType.h"

To reproduce on Windows, configure a build with -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON.

Why does Docker turn precompiled headers off?

The original Docker build PR (#1774) doesn't say why. The most likely reason is that VC6 precompiled headers are unreliable under Wine. VC6 saves a precompiled header as a raw copy of compiler memory and must load it back at the same address. Under Wine (and under ASLR on modern Windows) that often fails with C1859/C1852 errors, at random.

This turns out to be useful: a build without precompiled headers catches missing includes like these. The clang-tidy setup turns them off for the same reason.

To test whether precompiled headers now work under Wine, remove that flag and rebuild with FORCE_CMAKE=true.

Would a modern MSVC in Docker avoid this?

Only partly:

• Precompiled headers: modern MSVC handles them much better, but C1859 is still possible, so it isn't guaranteed under Wine.
• Purpose of the Docker build: it exists to produce VC6 binaries (ENV PRESET=vc6), which are needed for retail compatibility. Modern MSVC generates different code, especially floating-point code, so it can't replace VC6. It could only be an additional build.
• These errors: they would fail the same way with modern MSVC as long as precompiled headers are off.

Bottom line: fix the includes. The code is then correct with any compiler and build setup.

---

So it looks like it is better to include PreRTS rather than assume it is auto-included (precompiled).
Imho this means we should untangle PreRTS.h at some point and use proper includes for each file. PathFindCellInfo.h e.g. doesn't need all the stuff it pulls in.

[PedroTiago23]

I had claude have a go at it

Why PathfindCellInfo fails in the Docker build but compiles on Windows

Cause: The Docker build turns precompiled headers off (-DCMAKE_DISABLE_PRECOMPILE_HEADERS=1 in resources/dockerbuild/entrypoint.sh). On Windows, corei_gameengine_private force-includes PreRTS.h into every GameEngine .cpp as a precompiled header. That hides missing includes. Without it, each file has to include what it uses.

PathfindCellInfo.h has no includes, and PathfindCellInfo.cpp doesn't include PreRTS.h first. That explains every error:
Error Symbol Defined in
C2501 ICoord2D Lib/BaseType.h
C2501 ObjectID Common/GameType.h
C2065 MSGNEW Common/GameMemory.h
C2065 DEBUG_ASSERTCRASH Common/Debug.h

The compiler isn't the difference: the Docker image uses VC6 too. VC6's cl.exe reports itself as version 12.00, hence "MSVC 12".

Fix:

1. Start `PathfindCellInfo.cpp` with `#include "PreRTS.h"`, like every other GameEngine source file.

2. Make `PathfindCellInfo.h` self-contained:
   ```c++
   #pragma once
   
   #include "Lib/BaseType.h"
   #include "Common/GameType.h"
   ```

To reproduce on Windows, configure a build with -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON.

Why does Docker turn precompiled headers off?

The original Docker build PR (#1774) doesn't say why. The most likely reason is that VC6 precompiled headers are unreliable under Wine. VC6 saves a precompiled header as a raw copy of compiler memory and must load it back at the same address. Under Wine (and under ASLR on modern Windows) that often fails with C1859/C1852 errors, at random.

This turns out to be useful: a build without precompiled headers catches missing includes like these. The clang-tidy setup turns them off for the same reason.

To test whether precompiled headers now work under Wine, remove that flag and rebuild with FORCE_CMAKE=true.

Would a modern MSVC in Docker avoid this?

Only partly:

* **Precompiled headers:** modern MSVC handles them much better, but C1859 is still possible, so it isn't guaranteed under Wine.

* **Purpose of the Docker build:** it exists to produce **VC6** binaries (`ENV PRESET=vc6`), which are needed for retail compatibility. Modern MSVC generates different code, especially floating-point code, so it can't replace VC6. It could only be an additional build.

* **These errors:** they would fail the same way with modern MSVC as long as precompiled headers are off.

Bottom line: fix the includes. The code is then correct with any compiler and build setup.

So it looks like it is better to include PreRTS rather than assume it is auto-included (precompiled). Imho this means we should untangle PreRTS.h at some point and use proper includes for each file. PathFindCellInfo.h e.g. doesn't need all the stuff it pulls in.

That makes sense. I agree that, in the future, we should reduce the reliance on PreRTS.h and make header files, such as PathfindCellInfo.h, self-contained, including only the dependencies they actually need.

However, for this PR, I believe the current changes are sufficient. I ran a clean Docker/VC6 build, which completed successfully with the six Pathfinder source files including PreRTS.h, even without the additional includes in the four Pathfinder headers and without applying the build-system workaround from PR #3555.

I think it's best to keep this PR focused on the minimal, tested fix and leave the broader header-dependency cleanup for a separate future change.

[stephanmeesters]

See this comment #3555 (comment)

[PedroTiago23]

See this comment #3555 (comment)

Oh, okay. perfect.

[Skyaero42]

Doesn't match the title's description, but I think it fixes a bug reported here?
#3333 (comment)

[PedroTiago23]

This PR addresses the missing include convention exposed by the Pathfinder source split and the resulting Docker build failure. It doesn't attempt to fix the pathfinding behavior issues tracked in #3333; those broader changes should remain separate.

[Skyaero42]

Is this needed?

[PedroTiago23]

No, it’s redundant. PreRTS.h already includes "Common/GameDefines.h". I’ll remove it.

[Skyaero42]

This fixes an oversight from the deterministic math right?

[PedroTiago23]

Yes, this is related to the use of the project's "Sqrt" function. "ICoord2D::length()" calls "Sqrt(double)", which is declared in "trig.h", so the include is needed.

[Skyaero42]

What is this for?

[PedroTiago23]

These includes were left over from my earlier attempt to replace PreRTS.h with individual dependencies. Since PreRTS.h already provides them, they are redundant here. I'll remove them.