Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions Core/GameEngine/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -486,16 +486,16 @@ set(GAMEENGINE_SRC
# Include/GameLogic/ObjectScriptStatusBits.h
# Include/GameLogic/ObjectTypes.h
# Include/GameLogic/PartitionManager.h
# Include/GameLogic/PolygonTrigger.h
Include/GameLogic/PolygonTrigger.h
# Include/GameLogic/Powers.h
Include/GameLogic/RankInfo.h
# Include/GameLogic/ScriptActions.h
# Include/GameLogic/ScriptConditions.h
# Include/GameLogic/ScriptEngine.h
# Include/GameLogic/Scripts.h
# Include/GameLogic/SidesList.h
Include/GameLogic/SidesList.h
# Include/GameLogic/Squad.h
# Include/GameLogic/TerrainLogic.h
Include/GameLogic/TerrainLogic.h
# Include/GameLogic/TurretAI.h
# Include/GameLogic/VictoryConditions.h
# Include/GameLogic/Weapon.h
Expand Down Expand Up @@ -853,9 +853,9 @@ set(GAMEENGINE_SRC
# Source/GameLogic/AI/AITNGuard.cpp
# Source/GameLogic/AI/Squad.cpp
# Source/GameLogic/AI/TurretAI.cpp
# Source/GameLogic/Map/PolygonTrigger.cpp
# Source/GameLogic/Map/SidesList.cpp
# Source/GameLogic/Map/TerrainLogic.cpp
Source/GameLogic/Map/PolygonTrigger.cpp
Source/GameLogic/Map/SidesList.cpp
Source/GameLogic/Map/TerrainLogic.cpp
# Source/GameLogic/Object/Armor.cpp
# Source/GameLogic/Object/Behavior/AutoHealBehavior.cpp
# Source/GameLogic/Object/Behavior/BattleBusSlowDeathBehavior.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ class PolygonTrigger : public MemoryPoolObject,
Bool m_exportWithScripts;
Bool m_isWaterArea; ///< Used to specify water areas in the map.
Bool m_isRiver; ///< Used to specify that a water area is a river.
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
AsciiString m_layerName; ///< Used to specify the layer in the World Builder.
Bool m_shouldRender;
Bool m_selected;
#endif

static PolygonTrigger* ThePolygonTriggerListPtr;
static Int s_currentID; ///< Current id for new triggers.
Expand Down Expand Up @@ -119,6 +121,7 @@ class PolygonTrigger : public MemoryPoolObject,
void deletePoint(Int ndx);
void setTriggerName(AsciiString name) {m_triggerName = name;};

#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
void setLayerName(AsciiString name) {m_layerName = name;};
AsciiString getLayerName() const {return m_layerName;}

Expand All @@ -127,6 +130,7 @@ class PolygonTrigger : public MemoryPoolObject,

void setSelected(Bool toggle) {m_selected = toggle;}
Bool getSelected() {return m_selected;}
#endif

void getCenterPoint(Coord3D* pOutCoord) const;
Real getRadius() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,9 @@ class TerrainLogic : public Snapshot,
void setActiveBoundary(Int newActiveBoundary);

void flattenTerrain(Object *obj); ///< Flatten the terrain under a building.
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
void createCraterInTerrain(Object *obj); ///< Flatten the terrain under a building.
#endif

protected:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ m_numPoints(0),
m_sizePoints(0),
m_exportWithScripts(false),
m_isWaterArea(false),
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
m_shouldRender(true),
m_selected(false),
#endif
m_isRiver(FALSE),
m_riverStart(0)
{
Expand Down Expand Up @@ -142,7 +144,9 @@ Bool PolygonTrigger::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChu
Bool isRiver;
Int riverStart;
AsciiString triggerName;
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
AsciiString layerName;
#endif
// Remove any existing polygon triggers, if any.
PolygonTrigger::deleteTriggers(); // just in case.
PolygonTrigger *pPrevTrig = nullptr;
Expand All @@ -151,9 +155,11 @@ Bool PolygonTrigger::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChu
while (count>0) {
count--;
triggerName = file.readAsciiString();
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
if (info->version >= K_TRIGGERS_VERSION_4) {
layerName = file.readAsciiString();
}
Comment on lines +158 to 161

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Trigger v4 crc misparse 🐞 Bug ☼ Reliability

In PolygonTrigger::ParsePolygonTriggersDataChunk, the layer-name read for K_TRIGGERS_VERSION_4
is compiled out when RTS_GENERALS && RETAIL_COMPATIBLE_CRC, so a v4 PolygonTriggers chunk will
be parsed with a shifted read cursor and corrupt subsequent reads (likely leading to
crashes/asserts). This is especially risky because Zero Hour tooling writes PolygonTriggers as v4,
so cross-game/custom content can DOS the Generals build.
Agent Prompt
### Issue description
`PolygonTrigger::ParsePolygonTriggersDataChunk` conditionally compiles out reading the v4 `layerName` field for Generals retail-CRC builds. If a v4 chunk is encountered, the parser will interpret the layer-name bytes as `triggerID` and subsequent fields, corrupting parsing and likely crashing.

### Issue Context
- In Generals builds, `RTS_GENERALS=1` is set via CMake.
- `RETAIL_COMPATIBLE_CRC` defaults to 1 unless overridden.
- Zero Hour tooling/code paths write `PolygonTriggers` as version 4.

### Fix Focus Areas
- Core/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[137-184]
- Core/GameEngine/Source/GameLogic/Map/PolygonTrigger.cpp[250-268]

### What to change
- Make the parser *always* consume the v4 `layerName` string when `info->version >= K_TRIGGERS_VERSION_4`, even in `RTS_GENERALS && RETAIL_COMPATIBLE_CRC` builds (discard it if the member/method is compiled out).
- Alternatively (or additionally), explicitly reject unsupported versions in the CRC build (e.g., if `info->version > K_TRIGGERS_VERSION_3`, return false) rather than mis-parsing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

#endif
triggerID = file.readInt();
isWater = false;
if (info->version >= K_TRIGGERS_VERSION_2) {
Expand All @@ -169,9 +175,11 @@ Bool PolygonTrigger::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChu
numPoints = file.readInt();
PolygonTrigger *pTrig = newInstance(PolygonTrigger)(numPoints+1);
pTrig->setTriggerName(triggerName);
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
if (info->version >= K_TRIGGERS_VERSION_4) {
pTrig->setLayerName(layerName);
}
#endif
pTrig->setWaterArea(isWater);
pTrig->setRiver(isRiver);
pTrig->setRiverStart(riverStart);
Expand All @@ -186,12 +194,14 @@ Bool PolygonTrigger::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChu
loc.z = file.readInt();
pTrig->addPoint(loc);
}
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
if (numPoints<2) {
DEBUG_LOG(("Deleting polygon trigger '%s' with %d points.",
pTrig->getTriggerName().str(), numPoints));
deleteInstance(pTrig);
continue;
}
#endif
if (pPrevTrig) {
pPrevTrig->setNextPoly(pTrig);
} else {
Expand Down Expand Up @@ -239,7 +249,11 @@ Bool PolygonTrigger::ParsePolygonTriggersDataChunk(DataChunkInput &file, DataChu
*/
void PolygonTrigger::WritePolygonTriggersDataChunk(DataChunkOutput &chunkWriter)
{
#if RTS_GENERALS && RETAIL_COMPATIBLE_CRC
chunkWriter.openDataChunk("PolygonTriggers", K_TRIGGERS_VERSION_3);
#else
chunkWriter.openDataChunk("PolygonTriggers", K_TRIGGERS_VERSION_4);
#endif

PolygonTrigger *pTrig;
Int count = 0;
Expand All @@ -249,7 +263,9 @@ void PolygonTrigger::WritePolygonTriggersDataChunk(DataChunkOutput &chunkWriter)
chunkWriter.writeInt(count);
for (pTrig=PolygonTrigger::getFirstPolygonTrigger(); pTrig; pTrig = pTrig->getNext()) {
chunkWriter.writeAsciiString(pTrig->getTriggerName());
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
chunkWriter.writeAsciiString(pTrig->getLayerName());
#endif
chunkWriter.writeInt(pTrig->getID());
chunkWriter.writeByte(pTrig->isWaterArea());
chunkWriter.writeByte(pTrig->isRiver());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,23 +429,29 @@ static AsciiString static_readPlayerNames[MAX_PLAYER_COUNT];
* Input: DataChunkInput
*
*/
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
#define K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_1 1
#define K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2 2
#endif

static Bool ParsePlayersDataChunk(DataChunkInput &file, DataChunkInfo *info, void *userData)
{
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
Int readDicts = 0;
if (info->version >= K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2) {
readDicts = file.readInt();
Comment on lines +439 to 442

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Scriptsplayers v2 crc misparse 🐞 Bug ☼ Reliability

ParsePlayersDataChunk compiles out reading the v2 preamble/int+dict payload when `RTS_GENERALS &&
RETAIL_COMPATIBLE_CRC, so a v2 ScriptsPlayers` chunk (written by Zero Hour WorldBuilder) will be
misread (first int treated as numNames), leading to out-of-bounds reads and/or chunk-length
assertion failures. This creates a crash-on-load path for cross-game/custom .scb script files in
Generals retail-CRC builds.
Agent Prompt
### Issue description
In `ParsePlayersDataChunk`, when `RTS_GENERALS && RETAIL_COMPATIBLE_CRC` the code no longer reads the v2 `doSides/readDicts` int nor any side dicts. If a v2 `ScriptsPlayers` chunk is encountered, parsing becomes misaligned (the preamble int is treated as `numNames`), which can crash or hit `atEndOfChunk()` assertions.

### Issue Context
- Generals builds set `RTS_GENERALS=1`.
- `RETAIL_COMPATIBLE_CRC` defaults to 1.
- Zero Hour WorldBuilder writes `ScriptsPlayers` as version 2 and includes an extra int plus optional side dicts.

### Fix Focus Areas
- Core/GameEngine/Source/GameLogic/Map/SidesList.cpp[432-457]
- GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.cpp[1368-1381]

### What to change
- In the CRC build, still consume the v2 fields when `info->version >= 2`:
  - Always read/discard the `doSides/readDicts` int.
  - If that int indicates dicts are present, read/discard dict(s) to keep cursor alignment (even if Generals ignores them).
- Or explicitly reject v2+ chunks in CRC builds (return false) rather than mis-parsing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
#endif
Int numNames = file.readInt();
Int i;
for (i=0; i<numNames; i++) {
if (i>=MAX_PLAYER_COUNT) break;
static_readPlayerNames[i] = file.readAsciiString();
#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
if (readDicts) {
Dict sideDict = file.readDict();
}
#endif
}
DEBUG_ASSERTCRASH(file.atEndOfChunk(), ("Unexpected data left over."));
return true;
Expand Down Expand Up @@ -1131,7 +1137,11 @@ void TeamsInfoRec::addTeam(const Dict* d)
TEAM_ALLOC_CHUNK = 8 ///< how many teams to alloc at a time
};

#if RTS_GENERALS && RETAIL_COMPATIBLE_CRC
DEBUG_ASSERTCRASH(m_numTeams < 1024, ("hmm, seems like an awful lot of teams..."));
#else
DEBUG_ASSERTCRASH(m_numTeams < 2048, ("%d teams have been allocated (so far). This seems excessive.", m_numTeams ));
#endif
if (m_numTeams >= m_numTeamsAllocated)
{
// pool[]ify
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2866,8 +2866,7 @@ void TerrainLogic::flattenTerrain(Object *obj)

}



#if !(RTS_GENERALS && RETAIL_COMPATIBLE_CRC)
// ------------------------------------------------------------------------------------------------
/** Dig a deep circular gorge into the terrain beneath an object. */
// ------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -2916,12 +2915,7 @@ void TerrainLogic::createCraterInTerrain(Object *obj)
}

}






#endif

// ------------------------------------------------------------------------------------------------
/** CRC */
Expand Down
12 changes: 6 additions & 6 deletions Generals/Code/GameEngine/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -437,16 +437,16 @@ set(GAMEENGINE_SRC
Include/GameLogic/ObjectScriptStatusBits.h
Include/GameLogic/ObjectTypes.h
Include/GameLogic/PartitionManager.h
Include/GameLogic/PolygonTrigger.h
# Include/GameLogic/PolygonTrigger.h
Include/GameLogic/Powers.h
# Include/GameLogic/RankInfo.h
Include/GameLogic/ScriptActions.h
Include/GameLogic/ScriptConditions.h
Include/GameLogic/ScriptEngine.h
Include/GameLogic/Scripts.h
Include/GameLogic/SidesList.h
# Include/GameLogic/SidesList.h
Include/GameLogic/Squad.h
Include/GameLogic/TerrainLogic.h
# Include/GameLogic/TerrainLogic.h
Include/GameLogic/TurretAI.h
Include/GameLogic/VictoryConditions.h
Include/GameLogic/Weapon.h
Expand Down Expand Up @@ -787,9 +787,9 @@ set(GAMEENGINE_SRC
Source/GameLogic/AI/AITNGuard.cpp
Source/GameLogic/AI/Squad.cpp
Source/GameLogic/AI/TurretAI.cpp
Source/GameLogic/Map/PolygonTrigger.cpp
Source/GameLogic/Map/SidesList.cpp
Source/GameLogic/Map/TerrainLogic.cpp
# Source/GameLogic/Map/PolygonTrigger.cpp
# Source/GameLogic/Map/SidesList.cpp
# Source/GameLogic/Map/TerrainLogic.cpp
Source/GameLogic/Object/Armor.cpp
Source/GameLogic/Object/Behavior/AutoHealBehavior.cpp
Source/GameLogic/Object/Behavior/BehaviorModule.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#define K_TRIGGERS_VERSION_1 1
#define K_TRIGGERS_VERSION_2 2 // Added m_isWaterArea
#define K_TRIGGERS_VERSION_3 3 // Added m_isRiver & m_riverStart
#define K_TRIGGERS_VERSION_4 4 // Added layer name.
#define K_LIGHTING_VERSION_1 1
#define K_LIGHTING_VERSION_2 2 // Added 2 additional global lights for objects.
#define K_LIGHTING_VERSION_3 3 // Added 2 additional global lights for terrain.
Expand Down
Loading