bugfix(worldbuilder): Roll back failed blend allocations

[OmarAglan]

• Depends on unify(worldbuilder): Merge heightmap editing #3368.

Applies the same fix to the Generals and Zero Hour copies of WHeightMapEdit.cpp in their existing directories.

A three-way terrain blend can require both a secondary blend record and a flipped primary copy. If the secondary consumes the last available slot, the primary allocation fails and the current code stores -1 as the cell’s primary blend index.

Obtain both required records before changing the cell indices or cliff mapping. If the primary allocation fails, restore the previous blend-table count to reclaim any secondary record created by the failed attempt.

Existing records remain reusable when the table is full, and capacity warnings are preserved.

Validation:

• Reproduced the original invalid primary index.
• Passed 56 regression scenarios for each game copy, covering either allocation failing, full and nearly full tables, record reuse, missing source tiles and subsequent use of reclaimed slots.
• Compared successful results against the original production functions.
• Built both WorldBuilder targets with VC6 and passed debug/release compilation checks with data compatibility enabled and disabled.

This branch contains one fix commit above #3368 and is independent of the boundary-handle fix.

Codex generated the fix and local regression fixtures.

[coderabbitai]

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

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: 390885c1-c113-4b28-b935-2e2815a7e45b

📥 Commits

Reviewing files that changed from the base of the PR and between 01c7d68 and 8faff9a.

📒 Files selected for processing (1)

• Generals/Code/Tools/WorldBuilder/include/WHeightMapEdit.h

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

---

Walkthrough

The changes update height-map flag-row sizing, BlendTileData serialization, blend allocation handling, and boundary selection. They also remove getRawTileData, change directory image filename handling, and reset the boundary handle when no boundary is found.

Changes

Height-Map Editing

Layer / File(s)	Summary
Height-map data handling Generals/Code/Tools/WorldBuilder/include/WHeightMapEdit.h, Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp	Flag-row sizing now uses ceiling division by eight in construction and resize paths. Directory image loading copies each filename into the file buffer without adding the directory path. The getRawTileData declaration and implementation are removed. Comments for selectDuplicates and selectSimilar are corrected.
Blend serialization and allocation Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp, GeneralsMD/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp	BlendTileData serialization writes version 7 and legacy row widths when both build conditions are defined; otherwise, it writes version 8 and the full in-memory state. Blend allocation and cliff updates now occur within successful allocation paths.
Boundary selection and lookup Generals/Code/Tools/WorldBuilder/src/BorderTool.cpp, Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp	BorderTool::mouseDown handles motion == -1 as boundary creation and keeps the bottom-left boundary modification check. findBoundaryNear sets the output handle to -1 when no boundary is found.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: 🟡 Moderate · up to 8faff

Guard the optional boundary handle before merging: a boundary lookup with a null handle can crash when no boundary matches. The inspected boundary-tool caller is unaffected, and blend persistence matches the reader.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8faff

The change improves containment of failed terrain edits, and the supported saved-map formats match their reader. No new security boundary violation was established. Concurrent access and interruption recovery remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — In the inspected blend-tool path, editor coordinates and texture selection drive mutation of a duplicated working map's blend and cliff tables. The demonstrated scope is terrain state in the current editor document; this path does not show a tenant, service, credential, or privilege transition.

Trust Boundaries and Controls

• inferred — For synchronous edits of an unpublished map, delayed cell publication and count restoration prevent failed blend allocation from introducing a dangling primary record reference. Duplicate-map ownership is counterevidence to concurrent observation in the inspected caller; it is not a global thread-confinement guarantee.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: rolling back failed blend allocations in WorldBuilder.
Description check	✅ Passed	The description directly explains the blend-allocation bug, the rollback fix, affected game copies, dependencies, and validation results.
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.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{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: ec896c24-cf3c-4218-bd92-4499ae1811e7

📥 Commits

Reviewing files that changed from the base of the PR and between e6d72ec and 01c7d68.

📒 Files selected for processing (4)

• 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/src/WHeightMapEdit.cpp

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

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Check outHandle for null before writing to it.

The header says outHandle can be null: "outNdx must not be null, but outHandle can be." Each match branch checks if (outHandle) before it writes. The new fallback line writes through outHandle with no check. If a caller passes nullptr and no boundary is near, the editor dereferences a null pointer and crashes.

🐛 Proposed fix

 	(*outNdx) = -1;
-	(*outHandle) = -1;
+	if (outHandle) {
+		(*outHandle) = -1;
+	}

The early !pt return also resets only outNdx. For consistency, reset outHandle there as well when outHandle is not null.

📝 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

	(*outHandle) = -1;
	if (outHandle) {
	(*outHandle) = -1;
	}