bugfix(worldbuilder): Reject lossy retail cliff saves by OmarAglan · Pull Request #3454 · TheSuperHackers/GeneralsGameCode

OmarAglan · 2026-10-09T16:10:47Z

Addresses the cliff-state save findings raised on #3383 and #3384.

The retail Generals BlendTileData v7 format writes a shorter cliff-state row for some map widths. Nonzero flags in the omitted byte are lost when the map is reopened.

Reject saves that would lose these flags. Save and Save As validate before opening or replacing a file. Autosave validates before rotating backups and leaves the pending-autosave state set. The writer also rejects the map before writing any chunks.

Retail-compatible Generals still writes v7. Zero Hour and Generals without RETAIL_COMPATIBLE_DATA retain v8 behavior. Both source copies receive the same changes.

Validation:

Extracted production checks exercised widths 1–257, every cliff-row byte, three rows, both games, and DATA=0/1.
Verified rejection before the save/autosave file-operation boundary and preservation of pending autosave state.
All 16 actual-source VC6 compilations passed: WHeightMapEdit and WorldBuilderDoc, both games, debug/release, DATA=0/1.

coderabbitai · 2026-10-09T16:11:15Z

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: 5b6b79df-0b42-4f66-9997-25935ad7bbd5

📥 Commits

Reviewing files that changed from the base of the PR and between 2119cc9 and 8e4430b.

📒 Files selected for processing (6)

Generals/Code/Tools/WorldBuilder/include/WHeightMapEdit.h
Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp
Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp
GeneralsMD/Code/Tools/WorldBuilder/include/WHeightMapEdit.h
GeneralsMD/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp
GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp

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

Walkthrough

The Generals and GeneralsMD WorldBuilder variants now detect cliff-state bytes beyond the legacy serialized row width. Document saves are rejected with an error, autosaves are skipped, and direct height-map serialization throws before writing chunks when such bytes are present.

Changes

Cliff-state save validation

Layer / File(s)	Summary
Check cliff-state data before serialization `Generals/Code/Tools/WorldBuilder/include/WHeightMapEdit.h`, `Generals/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp`, `GeneralsMD/Code/Tools/WorldBuilder/include/WHeightMapEdit.h`, `GeneralsMD/Code/Tools/WorldBuilder/src/WHeightMapEdit.cpp`	Both variants add `canSaveCliffState()`. In retail-compatible Generals builds, it checks each row for nonzero bytes beyond the legacy row width. `saveToFile` throws `ERROR_CORRUPT_FILE_FORMAT` if the check fails.
Guard document saves and autosaves `Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp`, `GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp`	`DoSave` displays an error and returns `FALSE` when cliff state cannot be saved. `autoSave` skips writing an autosave under the same condition.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 8e443

No actionable issue remains in the supplied review evidence; the change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: rejecting retail WorldBuilder saves that would lose cliff state.
Description check		The description directly explains the cliff-state data-loss issue, the save and autosave validation changes, affected configurations, and reported validation results.
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.

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

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

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

greptile-apps · 2026-10-09T16:14:32Z

Confidence Score: 4/5

[Medium impact] World map editor adds validation to prevent lossy saves.

The PR appears safe to merge, with a non-blocking fix needed for the misleading autosave message.

Findings

Autosave reports false success ▶

Summary

The PR rejects retail Generals saves that would drop right-edge cliff flags.

WorldBuilder stops saves that would drop right-edge cliff flags.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Save or autosave] --> B{Cliff flags fit the selected format?}
  B -->|No| C[Stop before file operations]
  B -->|Yes| D[Open file or rotate backups]
  D --> E[Writer checks again]
  E --> F[Write map chunks]
  C --> G[Autosave caller still reports completion]

_{Reviews (1) · Last reviewed commit: "bugfix(worldbuilder): Reject lossy retai..." · Reviewed by Greptile}

greptile-apps · 2026-10-09T16:14:36Z

+	if (m_heightMap && !m_heightMap->canSaveCliffState()) {
+		return;


Autosave reports false success

When canSaveCliffState() rejects a map, autoSave() returns without saving, but CMainFrame::OnTimer() still displays “Auto Save Complete.” Later attempts repeat that message while the backups stay unchanged. Return a save result and show a skipped-save message when cliff flags cannot be stored. Apply this to both source copies.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e4430b82a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-09T16:15:11Z

 {
+	if (m_heightMap && !m_heightMap->canSaveCliffState()) {
+		::AfxMessageBox("Cannot save: the retail Generals map format cannot store cliff flags at the right edge of this map. Adjust the map width before saving.");
+		return FALSE;


Abort Jump to Game after rejecting the save

When an existing retail Generals map with a nonzero omitted cliff byte is launched through Ctrl+J, this new FALSE return reaches OnJumpToGame, whose call to DoFileSave() at line 620 ignores the result and still executes _spawnl at line 628. The editor therefore launches the previous on-disk map immediately after reporting that the current state cannot be saved, so a playtest can run against stale terrain; make that caller abort when the save fails.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-09T16:15:15Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-09T16:15:14.097305Z	`8e4430b`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

bugfix(worldbuilder): Reject lossy retail cliff saves

8e4430b

OmarAglan mentioned this pull request Oct 9, 2026

bugfix(worldbuilder): Roll back failed blend allocations #3384

Open

greptile-apps Bot reviewed Oct 9, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 9, 2026

View reviewed changes

OmarAglan marked this pull request as draft October 9, 2026 16:16

Navigation Menu

Repository navigation

bugfix(worldbuilder): Reject lossy retail cliff saves - #3454

bugfix(worldbuilder): Reject lossy retail cliff saves#3454
OmarAglan wants to merge 1 commit into
TheSuperHackers:mainfrom
OmarAglan:bugfix/worldbuilder-retail-cliff-save

OmarAglan commented Oct 9, 2026

Uh oh!

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 9, 2026 •

edited

Loading

Summary

Diagram

Uh oh!

greptile-apps Bot Oct 9, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 9, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		if (m_heightMap && !m_heightMap->canSaveCliffState()) {
		return;

Conversation

OmarAglan commented Oct 9, 2026

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

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

Uh oh!

Confidence Score: 4/5

Findings

Summary

Diagram

Uh oh!

greptile-apps Bot Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Oct 9, 2026

Codex Review Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

greptile-apps Bot commented Oct 9, 2026 •

edited

Loading