Skip to content

bugfix(scripts): Read Zero Hour script chunks and version 4 polygon triggers when importing scripts #3601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OmarAglan wants to merge 2 commits into
base: main
Choose a base branch
from
+67 −8
Open

bugfix(scripts): Read Zero Hour script chunks and version 4 polygon triggers when importing scripts #3601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c3db38
bugfix(scripts): Read Zero Hour script action and condition chunks in…
OmarAglan Oct 26, 2026
d929697
bugfix(worldbuilder): Read the layer name of version 4 polygon trigge…
OmarAglan Nov 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ enum { K_SCRIPT_LIST_DATA_VERSION_1 = 1,
K_SCRIPT_CONDITION_VERSION_2 = 2,
K_SCRIPT_CONDITION_VERSION_3 = 3,
K_SCRIPTS_DATA_VERSION_1,

// Added in Zero Hour
K_SCRIPT_ACTION_VERSION_2 = 2,
K_SCRIPT_CONDITION_VERSION_4 = 4,
end_of_the_enumeration
};

Expand Down Expand Up @@ -1658,7 +1662,18 @@ Bool Condition::ParseConditionDataChunk(DataChunkInput &file, DataChunkInfo *inf
Condition *pCondition = newInstance(Condition);
OrCondition *pOr = (OrCondition *)userData;
pCondition->m_conditionType = (enum ConditionType)file.readInt();
pCondition->m_numParms =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) {

@xezon xezon Nov 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@OmarAglan OmarAglan Nov 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@xezon xezon Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

file.readNameKey();
}
const Bool unknownType = pCondition->m_conditionType < 0 || pCondition->m_conditionType >= NUM_ITEMS;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
if (unknownType) {
Comment thread
bobtista marked this conversation as resolved.
Resolved
// The parameters of an unknown type are not read. Closing the chunk skips them.
DEBUG_LOG(("Unknown script condition %d. Making it false.", pCondition->m_conditionType));
pCondition->m_conditionType = CONDITION_FALSE;
} else {
pCondition->m_numParms =file.readInt();
}
Int i;
for (i=0; i<pCondition->m_numParms; i++)
{
Expand Down Expand Up @@ -1695,7 +1710,7 @@ Bool Condition::ParseConditionDataChunk(DataChunkInput &file, DataChunkInfo *inf
} else {
pOr->setFirstAndCondition(pCondition);
}
DEBUG_ASSERTCRASH(file.atEndOfChunk(), ("Unexpected data left over."));
DEBUG_ASSERTCRASH(unknownType || file.atEndOfChunk(), ("Unexpected data left over."));
return true;
}

Expand Down Expand Up @@ -2341,6 +2356,24 @@ void ScriptAction::WriteActionDataChunk(DataChunkOutput &chunkWriter, ScriptActi
}
}

// TheSuperHackers @bugfix Read the name key that Zero Hour writes after the script action type.
// The Generals types are the first entries of the Zero Hour lists, so the type number alone identifies them.
// Types past the end of the Generals list only exist in Zero Hour and become a no-op.
// The parameters of an unknown type are not read. Closing the chunk skips them.
static ScriptAction::ScriptActionType readActionType(DataChunkInput &file, DataChunkInfo *info, Script *pScript, Bool &unknownType)
{
Int type = file.readInt();
if (info->version >= K_SCRIPT_ACTION_VERSION_2) {
file.readNameKey();
}
unknownType = type < 0 || type >= ScriptAction::NUM_ITEMS;
if (unknownType) {
DEBUG_LOG(("Unknown script action %d in script '%s'. Making it a no-op.", type, pScript->getName().str()));
return ScriptAction::NO_OP;
}
return (ScriptAction::ScriptActionType)type;
}

/**
* ScriptAction::ParseActionDataChunk - read an action chunk in a script list.
* Format is the newer CHUNKY format.
Expand All @@ -2354,7 +2387,8 @@ Bool ScriptAction::ParseActionDataChunk(DataChunkInput &file, DataChunkInfo *inf

ScriptAction *pScriptAction = newInstance(ScriptAction);

pScriptAction->m_actionType = (enum ScriptActionType)file.readInt();
Bool unknownType;
pScriptAction->m_actionType = readActionType(file, info, pScript, unknownType);

#if defined(RTS_DEBUG)
const ActionTemplate* at = TheScriptEngine->getActionTemplate(pScriptAction->m_actionType);
Expand All @@ -2363,7 +2397,9 @@ Bool ScriptAction::ParseActionDataChunk(DataChunkInput &file, DataChunkInfo *inf
}
#endif

pScriptAction->m_numParms =file.readInt();
if (!unknownType) {
pScriptAction->m_numParms =file.readInt();
}
Int i;
for (i=0; i<pScriptAction->m_numParms; i++)
{
Expand Down Expand Up @@ -2449,7 +2485,7 @@ Bool ScriptAction::ParseActionDataChunk(DataChunkInput &file, DataChunkInfo *inf
{
pScript->setAction(pScriptAction);
}
DEBUG_ASSERTCRASH(file.atEndOfChunk(), ("Unexpected data left over."));
DEBUG_ASSERTCRASH(unknownType || file.atEndOfChunk(), ("Unexpected data left over."));
return true;
}

Expand Down Expand Up @@ -2489,8 +2525,11 @@ Bool ScriptAction::ParseActionFalseDataChunk(DataChunkInput &file, DataChunkInfo

ScriptAction *pScriptAction = newInstance(ScriptAction);

pScriptAction->m_actionType = (enum ScriptActionType)file.readInt();
pScriptAction->m_numParms =file.readInt();
Bool unknownType;
pScriptAction->m_actionType = readActionType(file, info, pScript, unknownType);
if (!unknownType) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
pScriptAction->m_numParms =file.readInt();
}
Int i;
for (i=0; i<pScriptAction->m_numParms; i++) {
pScriptAction->m_parms[i] = Parameter::ReadParameter(file);
Expand All @@ -2505,7 +2544,7 @@ Bool ScriptAction::ParseActionFalseDataChunk(DataChunkInput &file, DataChunkInfo
} else {
pScript->setFalseAction(pScriptAction);
}
DEBUG_ASSERTCRASH(file.atEndOfChunk(), ("Unexpected data left over."));
DEBUG_ASSERTCRASH(unknownType || file.atEndOfChunk(), ("Unexpected data left over."));
return true;
}

Expand Down
8 changes: 8 additions & 0 deletions Generals/Code/Tools/WorldBuilder/src/ScriptDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,7 @@ Bool ScriptDialog::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChunk
Int triggerID;
// Int maxTriggerId = 0;
AsciiString triggerName;
AsciiString layerName;
// Remove any existing polygon triggers, if any.
ScriptDialog *pThis = (ScriptDialog *)userData;
pThis->m_firstTrigger = nullptr;
Expand All @@ -1667,6 +1668,10 @@ Bool ScriptDialog::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChunk
count--;
Bool isWater = false;
triggerName = file.readAsciiString();
// TheSuperHackers @bugfix Read the layer name of version 4 triggers.
if (info->version >= K_TRIGGERS_VERSION_4) {
layerName = file.readAsciiString();
}
triggerID = file.readInt();
if (info->version >= K_TRIGGERS_VERSION_2) {
isWater = file.readByte();
Expand All @@ -1680,6 +1685,9 @@ Bool ScriptDialog::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChunk
numPoints = file.readInt();
PolygonTrigger *pTrig = newInstance(PolygonTrigger)(numPoints+1);
pTrig->setTriggerName(triggerName);
if (info->version >= K_TRIGGERS_VERSION_4) {
pTrig->setLayerName(layerName);
}
pTrig->setWaterArea(isWater);
pTrig->setRiver(isRiver);
pTrig->setRiverStart(riverStart);
Expand Down
12 changes: 12 additions & 0 deletions GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "EditGroup.h"
#include "EditParameter.h"
#include "ExportScriptsOptions.h"
#include "LayersList.h"
#include "Common/ThingFactory.h"
#include "WaypointOptions.h"
#include "Common/UnicodeString.h"
Expand Down Expand Up @@ -1560,11 +1561,14 @@ void ScriptDialog::OnLoad()
}
PolygonTrigger *pTrig;
PolygonTrigger *pNextTrig;
TheLayersList->disableUpdates();

@xezon xezon Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

for (pTrig=m_firstTrigger; pTrig; pTrig = pNextTrig) {
pNextTrig = pTrig->getNext();
pTrig->setNextPoly(nullptr);
PolygonTrigger::addPolygonTrigger(pTrig);
TheLayersList->addPolygonTriggerToLayersList(pTrig, pTrig->getLayerName());
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved

@xezon xezon Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
TheLayersList->enableUpdates();

ScriptList *scripts[MAX_PLAYER_COUNT];
Int count = ScriptList::getReadScripts(scripts);
Expand Down Expand Up @@ -1863,6 +1867,7 @@ Bool ScriptDialog::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChunk
Int triggerID;
// Int maxTriggerId = 0;
AsciiString triggerName;
AsciiString layerName;
// Remove any existing polygon triggers, if any.
ScriptDialog *pThis = (ScriptDialog *)userData;
pThis->m_firstTrigger = nullptr;
Expand All @@ -1874,6 +1879,10 @@ Bool ScriptDialog::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChunk
count--;
Bool isWater = false;
triggerName = file.readAsciiString();
// TheSuperHackers @bugfix Read the layer name of version 4 triggers.
if (info->version >= K_TRIGGERS_VERSION_4) {
Comment thread
xezon marked this conversation as resolved.
Resolved
layerName = file.readAsciiString();
}
triggerID = file.readInt();
if (info->version >= K_TRIGGERS_VERSION_2) {
isWater = file.readByte();
Expand All @@ -1887,6 +1896,9 @@ Bool ScriptDialog::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChunk
numPoints = file.readInt();
PolygonTrigger *pTrig = newInstance(PolygonTrigger)(numPoints+1);
pTrig->setTriggerName(triggerName);
if (info->version >= K_TRIGGERS_VERSION_4) {
pTrig->setLayerName(layerName);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
pTrig->setWaterArea(isWater);
pTrig->setRiver(isRiver);
pTrig->setRiverStart(riverStart);
Expand Down
Loading