unify(worldbuilder): Align script import and export #3409
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1100,6 +1100,8 @@ void ScriptDialog::scanForWaypointsAndTeams(Script *pScript, Bool doUnits, Bool | |
| } | ||
|
|
||
| #define K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_1 1 | ||
| // Added in Zero Hour | ||
| #define K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2 2 | ||
|
|
||
| /** Write out selected scripts, and possibly waypoints, trigger areas & teams. */ | ||
| void ScriptDialog::OnSave() | ||
|
|
@@ -1108,6 +1110,7 @@ void ScriptDialog::OnSave() | |
| Bool doTriggerAreas = true; | ||
| Bool doUnits = true; | ||
| Bool doAllScripts = true; | ||
| Bool doSides = true; | ||
| Int i; | ||
|
|
||
| ExportScriptsOptions optionsDlg; | ||
|
|
@@ -1118,6 +1121,7 @@ void ScriptDialog::OnSave() | |
| doUnits = optionsDlg.getDoUnits(); | ||
| doTriggerAreas = optionsDlg.getDoTriggers(); | ||
| doAllScripts = optionsDlg.getDoAllScripts(); | ||
| doSides = optionsDlg.getDoSides(); | ||
|
|
||
| Script *pScript = getCurScript(); | ||
| ScriptGroup *pGroup = getCurGroup(); | ||
|
|
@@ -1211,12 +1215,27 @@ void ScriptDialog::OnSave() | |
| ScriptList::WriteScriptsDataChunk(chunkWriter, scripts, numScriptLists); | ||
|
|
||
| /***************Players DATA ***************/ | ||
| #if RTS_GENERALS && RETAIL_COMPATIBLE_DATA | ||
| const DataChunkVersionType playersVersion = K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_1; | ||
| doSides = false; | ||
| #else | ||
| const DataChunkVersionType playersVersion = K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2; | ||
| #endif | ||
| chunkWriter.openDataChunk("ScriptsPlayers", playersVersion); | ||
| if (playersVersion >= K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2) { | ||
| chunkWriter.writeInt(doSides); | ||
| } | ||
| if (doAllScripts || doSides) { | ||
| chunkWriter.writeInt(m_sides.getNumSides()); | ||
| for (i=0; i<m_sides.getNumSides(); i++) { | ||
| AsciiString name = m_sides.getSideInfo(i)->getDict()->getAsciiString(TheKey_playerName); | ||
| chunkWriter.writeAsciiString(name); | ||
|
|
||
| if (doSides) { | ||
| // The user has requested that the sides get exported. | ||
| chunkWriter.writeDict(*m_sides.getSideInfo(i)->getDict()); | ||
| } | ||
|
|
||
| } | ||
| } else { | ||
| chunkWriter.writeInt(1); | ||
|
|
@@ -1374,8 +1393,16 @@ void ScriptDialog::OnLoad() | |
| file.registerParser( "WaypointsList", AsciiString::TheEmptyString, ParseWaypointDataChunk ); | ||
| file.registerParser( "ScriptTeams", AsciiString::TheEmptyString, ParseTeamsDataChunk ); | ||
| file.registerParser( "ScriptsPlayers", AsciiString::TheEmptyString, ParsePlayersDataChunk ); | ||
| // TheSuperHackers @bugfix OmarAglan Restore sides and teams when script parsing fails. | ||
| SidesList sidesBeforeImport; | ||
| sidesBeforeImport = m_sides; | ||
| try { | ||
| if (!file.parse(this)) { | ||
| throw(ERROR_CORRUPT_FILE_FORMAT); | ||
| } | ||
| } catch(...) { | ||
| m_sides = sidesBeforeImport; | ||
| throw; | ||
|
Comment on lines
+1403
to
+1405
There was a problem hiding this comment.
If a bundle contains waypoint links and then fails a player-count or dictionary-name check, parsing has already added those links to the document. This catch restores the sides but leaves links to waypoints that were never imported. Parsed script lists can also remain pending for the next import. The same rollback behavior occurs in the Zero Hour editor. Knowledge Base Used: Game development tools |
||
| } | ||
| pDoc->setNextWaypointID(m_maxWaypoint); | ||
|
|
||
|
|
@@ -1385,6 +1412,11 @@ void ScriptDialog::OnLoad() | |
| REF_PTR_RELEASE(pUndo); // belongs to pDoc now. | ||
| m_sides = *TheSidesList; | ||
|
|
||
| // TheSuperHackers @bugfix OmarAglan Add imported players at their map indices after parsing succeeds. | ||
| for (Int sideIndex = sidesBeforeImport.getNumSides(); sideIndex < m_sides.getNumSides(); sideIndex++) { | ||
| addPlayer(sideIndex); | ||
| } | ||
|
|
||
| if (m_firstReadObject) { | ||
| AddObjectUndoable *pUndo = new AddObjectUndoable(pDoc, m_firstReadObject); | ||
| pDoc->AddAndDoUndoable(pUndo); | ||
|
|
@@ -1409,9 +1441,10 @@ void ScriptDialog::OnLoad() | |
| curSide = m_curSelection.m_playerIndex; | ||
| } else { | ||
| Int j; | ||
| // TheSuperHackers @bugfix OmarAglan Match each imported player to the current map side by name. | ||
| for (j=0; j<m_sides.getNumSides(); j++) { | ||
| AsciiString name = m_sides.getSideInfo(j)->getDict()->getAsciiString(TheKey_playerName); | ||
| if (name == m_readPlayerNames[i]) { | ||
| curSide = j; | ||
| break; | ||
| } | ||
|
|
@@ -1455,15 +1488,23 @@ void ScriptDialog::OnLoad() | |
| scripts[i]->discard(); /* Frees the script list, but none of it's children, as they have been | ||
| copied into the current scripts. */ | ||
| scripts[i] = nullptr; | ||
| //reloadPlayer(curSide, pSL); | ||
| } else { | ||
| deleteInstance(scripts[i]); | ||
| scripts[i] = nullptr; | ||
| } | ||
| } | ||
|
|
||
| for (i = 0; i < m_sides.getNumSides(); i++) { | ||
| // Make sure that the dialog tree is updated. | ||
| ScriptList *pSL = m_sides.getSideInfo(i)->getScriptList(); | ||
| reloadPlayer(i, pSL); | ||
| updateIcons(TVI_ROOT); | ||
| } | ||
|
|
||
|
|
||
| } catch(...) { | ||
| ::AfxMessageBox("Unable to import scripts. The file contains invalid data or exceeds the player limit.", MB_OK); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1607,15 +1648,19 @@ Bool ScriptDialog::ParseTeamsDataChunk(DataChunkInput &file, DataChunkInfo *info | |
| TeamsInfo ti; | ||
| ti.init(&teamDict); | ||
| CFixTeamOwnerDialog fix(&ti, &pThis->m_sides); | ||
| bool nameSet = false; | ||
| if (fix.DoModal() == IDOK) { | ||
| if (fix.pickedValidTeam()) { | ||
| teamDict.setAsciiString(TheKey_teamOwner, fix.getSelectedOwner()); | ||
| nameSet = true; | ||
| } | ||
| } | ||
|
|
||
| if (nameSet == false) { | ||
| AsciiString neutralPlayerName; // neutral player name is empty string | ||
| // player doesn't exist, so add it to the neutral player. | ||
| teamDict.setAsciiString(TheKey_teamOwner, neutralPlayerName); | ||
| } | ||
| pThis->m_sides.addTeam(&teamDict); | ||
| } | ||
| } | ||
|
|
@@ -1632,14 +1677,54 @@ Bool ScriptDialog::ParseTeamsDataChunk(DataChunkInput &file, DataChunkInfo *info | |
| Bool ScriptDialog::ParsePlayersDataChunk(DataChunkInput &file, DataChunkInfo *info, void *userData) | ||
| { | ||
| ScriptDialog *pThis = (ScriptDialog *)userData; | ||
| Int readDicts = 0; | ||
| if (info->version >= K_PLAYERS_NAMES_FOR_SCRIPTS_VERSION_2) { | ||
| readDicts = file.readInt(); | ||
| } | ||
| Int numNames = file.readInt(); | ||
| // TheSuperHackers @bugfix OmarAglan Reject player counts that cannot fit in the import array. | ||
| if (numNames < 0 || numNames > MAX_PLAYER_COUNT) { | ||
| return false; | ||
| } | ||
| Int i; | ||
| for (i=0; i<numNames; i++) { | ||
| pThis->m_readPlayerNames[i] = file.readAsciiString(); | ||
| if (readDicts) { | ||
| Dict sideDict = file.readDict(); | ||
| // TheSuperHackers @bugfix OmarAglan Validate the dictionary identity before adding a player. | ||
| Bool hasPlayerName; | ||
| AsciiString playerName = sideDict.getAsciiString(TheKey_playerName, &hasPlayerName); | ||
| if (!hasPlayerName || playerName != pThis->m_readPlayerNames[i]) { | ||
| return false; | ||
| } | ||
| bool nameFound = false; | ||
| for (Int j=0; j < pThis->m_sides.getNumSides(); j++) { | ||
| AsciiString name = pThis->m_sides.getSideInfo(j)->getDict()->getAsciiString(TheKey_playerName); | ||
|
|
||
| if (name == pThis->m_readPlayerNames[i]) { | ||
| // The side already exists so don't add it or overwrite the old data. | ||
| nameFound = true; | ||
| break; | ||
| } | ||
| } | ||
| if (nameFound == false) { | ||
| // TheSuperHackers @bugfix OmarAglan Check capacity before adding a side and using its script list. | ||
| if (pThis->m_sides.getNumSides() >= MAX_PLAYER_COUNT) { | ||
| return false; | ||
| } | ||
| // This side doesn't currently exist, so add it. | ||
| pThis->m_sides.addSide(&sideDict); | ||
| SidesInfo* sides = pThis->m_sides.findSideInfo(playerName); | ||
| if (sides == nullptr) { | ||
| return false; | ||
| } | ||
| ScriptList* pList = newInstance(ScriptList); | ||
| // A script list must be created. | ||
| sides->setScriptList(pList); | ||
| } | ||
| } | ||
| } | ||
| return file.atEndOfChunk(); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 High
src/ScriptDialog.cpp:1404A parse failure leaves imported waypoint links in the active document, so a file with valid waypoints followed by invalid player/script data produces a partial import that can be saved.
ParseWaypointDataChunkcallspDoc->addWaypointLink(...)duringfile.parse(this), but this catch block restores onlym_sides; roll back those document-side changes as well, or make the import transactional.🤖 Copy this AI Prompt to have your agent fix this: