bugfix(scripts): Read Zero Hour script chunks and version 4 polygon triggers when importing scripts by OmarAglan · Pull Request #3601 · TheSuperHackers/GeneralsGameCode

OmarAglan · 2026-10-26T10:58:43Z

Zero Hour writes ScriptAction and ScriptActionFalse chunks as version 2 and Condition chunks as version 4. These versions add a name key after the type. The Generals readers ignored the chunk version and read this name key as the parameter count, so opening a Zero Hour map in the Generals World Builder loaded broken scripts.

This change reads and skips the name key in all three readers.

The Generals script action and condition types are the first entries of the Zero Hour lists, in the same order, so the type number alone identifies them. Types past the end of the Generals list only exist in Zero Hour. They are turned into NO_OP actions or CONDITION_FALSE conditions. Their parameters are not read; closing the chunk skips them.

The new version constants are added after K_SCRIPTS_DATA_VERSION_1 so its implicit value, which is written as the PlayerScriptsList version, does not change.

Writing is unchanged. Generals still writes action version 1 and condition version 3.

Polygon triggers in script import

The script import in the World Builder (ScriptDialog::ParsePolygonTriggersDataChunk) has its own polygon trigger reader. It did not read the layer name that version 4 polygon triggers add after the trigger name, so importing scripts from a map with version 4 triggers (for example water areas from Zero Hour maps) read the rest of each trigger at the wrong position. This is a retail bug in both World Builders.

The reader now reads the layer name for version 4 triggers and sets it on the trigger, like PolygonTrigger::ParsePolygonTriggersDataChunk does. In the Zero Hour World Builder, the imported triggers are also added to the layer list, the same way map loading does it.

AI USE

Used AI in implementing the fix after several rounds of review and simplifying

chatgpt-codex-connector · 2026-10-26T10:58:53Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-26T10:58:48.214111Z We couldn’t complete this review. Comment `@codex review` to try again.	`e25cfab`	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.

chatgpt-codex-connector · 2026-10-26T10:58:56Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

coderabbitai · 2026-10-26T10:59:08Z

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.

Last reviewed commit: 05c1c36
Latest commit, not reviewed: d929697

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

Walkthrough

Script loading now reads versioned name keys and handles out-of-range condition and action types. Both WorldBuilder variants read layer names from version 4 or later polygon-trigger chunks and assign them to parsed polygon triggers.

Changes

Script chunk parsing

Layer / File(s)	Summary
Condition chunk parsing `Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp`	Condition parsing reads the version 4 name key. It maps out-of-range types to `CONDITION_FALSE` and permits their payloads to remain unread.
Action type parsing `Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp`	Normal and false-action parsing use a shared helper to read action types and version 2 name keys. Out-of-range types map to `NO_OP`, and their payloads can remain unread.

Polygon trigger import

Layer / File(s)	Summary
Import polygon-trigger layer names `Generals/Code/Tools/WorldBuilder/src/ScriptDialog.cpp`, `GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.cpp`	Both parsers read and assign layer names for version 4 or later trigger chunks. Older chunks do not use this layer-name field.

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

Change: Bug fix

Possibly related PRs

TheSuperHackers/GeneralsGameCode#3578: Both changes modify script data parsing, including how parameter counts are read for condition and action chunks.

Suggested reviewers: xezon

Merge Risk: Moderate · previous review 05c1c

Resolve parameter-count validation and name-key handling before merging: affected imports can corrupt memory or execute the wrong script behavior. Imported trigger layers also remain inconsistent in Zero Hour’s layer controls and are lost during script export.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: reading Zero Hour script chunks and version 4 polygon triggers during script import in the World Builder. The changeset matches this. The Scripts.cpp changes add name-…
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 states the problem and fix for the changeset: Zero Hour ScriptAction, ScriptActionFalse, and Condition chunks add a name key that the Generals readers misread, and the change reads and…

Full details: Title check

Explanation

The title names the main change: reading Zero Hour script chunks and version 4 polygon triggers during script import in the World Builder. The changeset matches this. The Scripts.cpp changes add name-key handling for action version 2 and condition version 4, and ScriptDialog.cpp reads the layer name for version 4 polygon triggers. The title uses a conventional bugfix prefix and is specific.

Full details: Description check

Explanation

The description states the problem and fix for the changeset: Zero Hour ScriptAction, ScriptActionFalse, and Condition chunks add a name key that the Generals readers misread, and the change reads and skips it in all three readers. It also covers the NO_OP/CONDITION_FALSE mapping for Zero Hour-only types, the version constants, the unchanged writing behavior, and the ScriptDialog::ParsePolygonTriggersDataChunk layer-name fix. This matches the Scripts.cpp and ScriptDialog.cpp summaries and the PR objectives, so the description is specific and on-topic.

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

greptile-apps · 2026-10-26T11:02:20Z

Confidence Score: 5/5

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

Summary

This PR lets Generals read Zero Hour script chunks and fixes version 4 polygon-trigger imports in both World Builders.

Generals scripts keep their fields in order when reading Zero Hour chunks.
Imported version 4 triggers keep their layers and fields.

_{Reviews (7) · Last reviewed commit: "bugfix(worldbuilder): Read the layer nam..." · Reviewed by Greptile}

xezon

I think it needs some consideration as to what Zero Hour does on the other side. Maybe Zero Hour code needs to be merged first? What is the plan?

xezon · 2026-11-07T19:30:18Z

 		count--;
 		Bool isWater = false;
 		triggerName = file.readAsciiString();
+		// TheSuperHackers @bugfix Read the layer name of version 4 triggers, like PolygonTrigger::ParsePolygonTriggersDataChunk.


Is the code reference necessary here?It adds text bloat. What was the user facing consequence of this not loaded?

done removed the reference

and to answer "What was the user facing consequence of this not loaded?"

before this fix, importing scripts in World Builder from a file with version 4 polygon triggers skipped the layer name, so the trigger ID, water and river flags, point count and points were all read from the wrong position. The imported trigger areas came in broken

xezon · 2026-11-07T20:04:49Z

 	Int i;
 	for (i=0; i<pCondition->m_numParms; i++)
 	{
 		pCondition->m_parms[i] = Parameter::ReadParameter(file);


Instead of allocating params here to just throw them away in the next op, try seek forward the DataChunkInput file pointer and avoid the allocation and.

i think your comment is not complete!
as far as i understood it, i edited it and now the unknown types no longer read their params

DataChunkInput::parse closes the chunk after the parser returns, and closeDataChunk seeks over the unread bytes.

now the "Unexpected data left over" assert now allows this case.

xezon · 2026-11-07T20:15:37Z

 	OrCondition *pOr = (OrCondition *)userData;
 	pCondition->m_conditionType = (enum ConditionType)file.readInt();
+	// TheSuperHackers @bugfix Skip the name key that Zero Hour writes after the type. See readActionType.
+	if (info->version >= K_SCRIPT_CONDITION_VERSION_4) {


Shouldn't we merge the code from ZH first or what is the plan here?

ZH code will be colliding with the new changes made here.

zh reader cant be taken as is yet, it matches the type again through m_internalNameKey, which gen does not have

this PR follows the same layout as zh

name key read right after the type, CONDITION_FALSE or NO_OP for unknown types

to know when Scripts.cpp moves to Core this code is simply replaced by the zh version.

thats being said, if you want and rather unify Scripts.cpp first, i can draft this pr or make another one that do that.

Yes maybe we should merge the code first and then mitigate the data serialization edge cases?

coderabbitai

Actionable comments posted: 5

Review info

Run configuration

Source mode: Auto

Repository: Unavailable

Branch: Unavailable

File: No configuration file from this repository applied

Commit: Unavailable

Layers: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: `33ab4127-55f6-420a-b99b-7eb722cb246c`

Commits

Reviewing files that changed from the base of the PR and between `3717535` and `05c1c36`.

Files selected for processing: 3

`Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp`

`Generals/Code/Tools/WorldBuilder/src/ScriptDialog.cpp`

`GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.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.

bobtista · 2026-11-08T23:57:30Z

The description still says the parameters of unknown types are read and then deleted. The code now leaves them for the chunk close to skip. Can you update that sentence?

OmarAglan · 2026-11-09T11:01:44Z

The description still says the parameters of unknown types are read and then deleted. The code now leaves them for the chunk close to skip. Can you update that sentence?

i tend to leave the pr title and description update as last step, as it can change multiple times, that's being said, i updated the pr description nonetheless.

… Generals

…rs when importing scripts

xezon · 2026-11-09T19:45:05Z

 		}
 		PolygonTrigger *pTrig;
 		PolygonTrigger *pNextTrig;
+		TheLayersList->disableUpdates();


What does this do?

xezon · 2026-11-09T19:47:01Z

 			pNextTrig = pTrig->getNext();
 			pTrig->setNextPoly(nullptr);
 			PolygonTrigger::addPolygonTrigger(pTrig);
+			TheLayersList->addPolygonTriggerToLayersList(pTrig, pTrig->getLayerName());


Why is this added? Generals still does not do this.

xezon · 2026-11-09T19:52:16Z

 	OrCondition *pOr = (OrCondition *)userData;
 	pCondition->m_conditionType = (enum ConditionType)file.readInt();
+	// TheSuperHackers @bugfix Skip the name key that Zero Hour writes after the type. See readActionType.
+	if (info->version >= K_SCRIPT_CONDITION_VERSION_4) {


Yes maybe we should merge the code first and then mitigate the data serialization edge cases?

greptile-apps Bot reviewed Oct 26, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp Resolved

OmarAglan changed the title ~~bugfix(scripts): Read Zero Hour script action and condition chunks in Generals~~ bugfix(scripts): Read Zero Hour script chunks and version 4 polygon triggers when importing scripts Oct 26, 2026

Caball009 self-requested a review October 29, 2026 03:08

tintinhamans added WorldBuilder Relates to World Builder Script Is related to Script Engine, SCB labels Oct 29, 2026

xezon reviewed Nov 7, 2026

View reviewed changes

OmarAglan force-pushed the bugfix/scripts-read-zh-script-chunks branch from 3717535 to 05c1c36 Compare November 8, 2026 11:26

coderabbitai Bot reviewed Nov 8, 2026

View reviewed changes

OmarAglan force-pushed the bugfix/scripts-read-zh-script-chunks branch 2 times, most recently from 2c3db16 to 93bfddc Compare November 8, 2026 12:39

greptile-apps Bot reviewed Nov 8, 2026

View reviewed changes

Comment thread GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.cpp Resolved

OmarAglan force-pushed the bugfix/scripts-read-zh-script-chunks branch from 93bfddc to 84f9924 Compare November 8, 2026 12:47

bobtista reviewed Nov 8, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp Resolved

OmarAglan added 2 commits November 9, 2026 14:02

bugfix(scripts): Read Zero Hour script action and condition chunks in…

6c3db38

… Generals

bugfix(worldbuilder): Read the layer name of version 4 polygon trigge…

d929697

…rs when importing scripts

OmarAglan force-pushed the bugfix/scripts-read-zh-script-chunks branch from 84f9924 to d929697 Compare November 9, 2026 11:02

xezon reviewed Nov 9, 2026

View reviewed changes

Conversation

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

Uh oh!

Polygon triggers in script import

AI USE

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Oct 26, 2026

Uh oh!

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

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

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

Uh oh!

Confidence Score: 5/5

Summary

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OmarAglan Nov 8, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobtista commented Nov 8, 2026

Uh oh!

Uh oh!

OmarAglan commented Nov 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

OmarAglan commented Oct 26, 2026 •

edited

Loading

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

greptile-apps Bot commented Oct 26, 2026 •

edited

Loading

OmarAglan Nov 8, 2026 •

edited

Loading