unify(worldbuilder): Merge and move heightmap editing to Core

[OmarAglan]

• Next Step after unify(thingtemplate): Merge and move ThingTemplate to Core #3324

This PR align WorldHeightMapEdit and BorderTool with Zero Hour, and moves their headers and sources to Core.

Both WorldBuilder targets compile the shared sources through corei_worldbuilder.

Generals Gets

• Zero Hour cliff-flag allocation
• Texture filename handling
• Shared blend record handling
• Boundary creation behavior.

Explain some points

The duplicate WorldHeightMapEdit::getRawTileData() is removed because the public base class implementation is identical and already used by Zero Hour.

also Generals still writes BlendTileData v7 with RETAIL_COMPATIBLE_DATA, packing each row to the legacy width. Other configurations write v8, that is the only game specific compatibility guard added here.

The shared blend path checks whether the copied base blend could be allocated before updating tile indices. Boundary lookup also checks its optional handle pointer when reporting a miss.

The alignment and mechanical move are separate commits for review.

Validation:

• Both WorldBuilder targets built successfully with VC6 before and after the move.
• Both changed source files compiled for each game in debug/release with data compatibility enabled and disabled.
• Extracted-source fixtures passed for v7/v8 payload compatibility, blend sharing and allocation failure, and border picking/dragging.

Codex assisted with the compatibility analysis, local validation.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Walkthrough

The build configuration adds a Core WorldBuilder target and links it to the Generals and Zero Hour tools. The changes also update height-map editing behavior and remove the BorderTool implementation and declarations.

Changes

WorldBuilder Core Migration

Layer / File(s)	Summary
Core height-map editor and behavior Core/Tools/CMakeLists.txt, Core/Tools/WorldBuilder/CMakeLists.txt, Core/Tools/WorldBuilder/src/WHeightMapEdit.cpp	The Core build exposes WorldBuilder headers and sources through an interface target. Height-map editing selects blend-tile chunk version 7 for the specified Generals retail-compatible build and version 8 otherwise. The editor also changes flipped-blend allocation handling and sets the optional boundary handle to -1 when no boundary is found.
Tool integration and legacy source removal Generals/Code/Tools/WorldBuilder/..., GeneralsMD/Code/Tools/WorldBuilder/CMakeLists.txt, scripts/cpp/unify_move_files.py	The Generals and Zero Hour tool targets link corei_worldbuilder and remove legacy source-list entries. The BorderTool files are deleted. The file-move script adds commented-out entries for the WorldBuilder files.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Suggested reviewers: xezon

Merge Risk: 🟡 Moderate · up to 0b0b3

Terrain editing near the blend-record limit can consume a slot without applying the edit. Make the allocations transactional before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0b0b3

Both editors continue to save maps through their existing document flows, and the reviewed file-format change matches the available reader. No new security exposure was established. Coverage of other consumers and runtime behavior remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is map data written by the two WorldBuilder editors and subsequently read by the available map loader. A broader consumer or deployment exposure was not established by the reviewed evidence.

Trust Boundaries and Controls

• observed — The document callers retain control of the output stream, while the reader distinguishes the versioned cliff-state payloads. The reviewed writer change does not itself select a file destination or add an input path.

Hardening Proposals

• proposed — If saved-map integrity under blend-table exhaustion is a requirement, consider preparing both blend records before mutation or reclaiming an unreferenced record on abort. This is hardening of a transition also present before the Core move, not a verified new security finding.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: aligning WorldBuilder behavior and moving heightmap editing to Core.
Description check	✅ Passed	The description directly explains the WorldBuilder alignment, source move, compatibility behavior, validation, and shared build setup.
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.}

[greptile-apps]

Confidence Score: 4/5

[High risk] Build configuration and tool source reorganization across game variants.

The PR appears safe to merge, with a non-blocking blend-table cleanup issue worth addressing.

Findings

1.  Unused blend consumes final slot ▶

Summary

The PR moves both WorldBuilder targets onto shared height-map and border-tool sources, while retaining Generals’ legacy BlendTileData output when retail compatibility is enabled.

• Adds a Core interface target and removes the duplicate game-specific sources.
• Aligns Generals’ cliff allocation, blend handling, texture filenames, and boundary creation with Zero Hour.
• The new blend-allocation failure path can retain an unused blend-table entry.

_{Reviews (1) · Last reviewed commit: "unify(worldbuilder): Move heightmap edit..."}

[greptile-apps]

Unused blend consumes final slot When a three-way blend needs a flipped base copy and only one blend-table slot remains, the secondary blend takes that slot first. Creating the base copy then fails, so the function returns without assigning the secondary to a cell. The unused record keeps the slot occupied, preventing a later blend from using it.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7110ebec-9419-478b-87ed-b4b5e144e523

📥 Commits

Reviewing files that changed from the base of the PR and between f8d16bb and 0b0b300.

📒 Files selected for processing (13)

• Core/Tools/CMakeLists.txt
• Core/Tools/WorldBuilder/CMakeLists.txt
• Core/Tools/WorldBuilder/include/BorderTool.h
• Core/Tools/WorldBuilder/include/WHeightMapEdit.h
• Core/Tools/WorldBuilder/src/BorderTool.cpp
• Core/Tools/WorldBuilder/src/WHeightMapEdit.cpp
• Generals/Code/Tools/WorldBuilder/CMakeLists.txt
• Generals/Code/Tools/WorldBuilder/include/BorderTool.h
• Generals/Code/Tools/WorldBuilder/include/WHeightMapEdit.h
• Generals/Code/Tools/WorldBuilder/src/BorderTool.cpp
• Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp
• GeneralsMD/Code/Tools/WorldBuilder/CMakeLists.txt
• scripts/cpp/unify_move_files.py

💤 Files with no reviewable changes (3)

• Generals/Code/Tools/WorldBuilder/include/BorderTool.h
• Generals/Code/Tools/WorldBuilder/src/BorderTool.cpp
• Generals/Code/Tools/WorldBuilder/include/WHeightMapEdit.h

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

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

rg -n 'findOrCreateBlendTile|m_numBlendTiles|MAX_BLEND|tempBlendTileInfo' Core/Tools/WorldBuilder/src/WHeightMapEdit.cpp
sed -n '1120,1210p' Core/Tools/WorldBuilder/src/WHeightMapEdit.cpp

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 4552

---

Do not retain the secondary blend record when primary allocation fails.

If the blend-record capacity is nearly exhausted, findOrCreateBlendTile(&blendInfo) can allocate a new secondary record. The subsequent findOrCreateBlendTile(&tempBlendTileInfo) can then return -1. The early return leaves the secondary record unreferenced and consumes a blend-record slot.

Make the two allocations transactional, or roll back the secondary record created by this call when primary allocation fails.