From 22b0518a9a507d8e3ed0cf725e461c8c3c45d03a Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 24 Dec 2023 12:10:51 +0300 Subject: [PATCH] Switch droid groups to using `std::list` Remove `psGrpNext` from `DROID` class, add `DroidList` field to the `DROID_GROUP` class instead. Introduce a utility function `mutating_list_iterate` in `objmem.h` which is a for-each-style algorithm to work with `std::list`, which can cope with invalidation of the current iterator (this can come handy in cases where we traverse the group list and remove/add from/to the same group list along the way). "mutating" word here clearly denotes that this loop may modify the list, which will be traversed. Signed-off-by: Pavel Solodovnikov --- src/ai.cpp | 4 +-- src/droid.cpp | 28 ++++++++------- src/droiddef.h | 1 - src/game.cpp | 8 ++--- src/group.cpp | 68 +++++++----------------------------- src/group.h | 11 +++--- src/loop.cpp | 8 +++-- src/mission.cpp | 85 ++++++++++++++++++++++++++++----------------- src/multibot.cpp | 18 +++++----- src/objmem.cpp | 15 +++----- src/objmem.h | 27 ++++++++++++++ src/order.cpp | 15 ++++---- src/research.cpp | 5 +-- src/scores.cpp | 2 +- src/transporter.cpp | 50 ++++++++++++++------------ src/wzapi.cpp | 3 +- 16 files changed, 180 insertions(+), 168 deletions(-) diff --git a/src/ai.cpp b/src/ai.cpp index e67050c4509..a2e42341792 100644 --- a/src/ai.cpp +++ b/src/ai.cpp @@ -273,7 +273,7 @@ static SDWORD targetAttackWeight(BASE_OBJECT *psTarget, BASE_OBJECT *psAttacker, { SDWORD targetTypeBonus = 0, damageRatio = 0, attackWeight = 0, noTarget = -1; UDWORD weaponSlot; - DROID *targetDroid = nullptr, *psAttackerDroid = nullptr, *psGroupDroid, *psDroid; + DROID *targetDroid = nullptr, *psAttackerDroid = nullptr, *psDroid; STRUCTURE *targetStructure = nullptr; WEAPON_EFFECT weaponEffect; WEAPON_STATS *attackerWeapon; @@ -521,7 +521,7 @@ static SDWORD targetAttackWeight(BASE_OBJECT *psTarget, BASE_OBJECT *psAttacker, } //fire support - go through all droids assigned to the commander - for (psGroupDroid = psAttackerDroid->psGroup->psList; psGroupDroid; psGroupDroid = psGroupDroid->psGrpNext) + for (const DROID* psGroupDroid : psAttackerDroid->psGroup->psList) { for (weaponSlot = 0; weaponSlot < psGroupDroid->numWeaps; weaponSlot++) { diff --git a/src/droid.cpp b/src/droid.cpp index 6ca50575c8c..7cc981ffbec 100644 --- a/src/droid.cpp +++ b/src/droid.cpp @@ -143,7 +143,7 @@ static void droidBodyUpgrade(DROID *psDroid) psDroid->baseSpeed = calcDroidBaseSpeed(&sTemplate, psDroid->weight, psDroid->player); if (isTransporter(psDroid)) { - for (DROID *psCurr = psDroid->psGroup->psList; psCurr != nullptr; psCurr = psCurr->psGrpNext) + for (DROID *psCurr : psDroid->psGroup->psList) { if (psCurr != psDroid) { @@ -387,7 +387,6 @@ DROID::DROID(uint32_t id, unsigned player) : BASE_OBJECT(OBJ_DROID, id, player) , droidType(DROID_ANY) , psGroup(nullptr) - , psGrpNext(nullptr) , secondaryOrder(DSS_ARANGE_LONG | DSS_REPLEV_NEVER | DSS_ALEV_ALWAYS | DSS_HALT_GUARD) , secondaryOrderPending(DSS_ARANGE_LONG | DSS_REPLEV_NEVER | DSS_ALEV_ALWAYS | DSS_HALT_GUARD) , secondaryOrderPendingCount(0) @@ -438,16 +437,18 @@ DROID::~DROID() audio_RemoveObj(this); DROID *psDroid = this; - DROID *psCurr, *pNextGroupDroid = nullptr; if (isTransporter(psDroid)) { if (psDroid->psGroup) { //free all droids associated with this Transporter - for (psCurr = psDroid->psGroup->psList; psCurr != nullptr && psCurr != psDroid; psCurr = pNextGroupDroid) + for (DROID* psCurr : psDroid->psGroup->psList) { - pNextGroupDroid = psCurr->psGrpNext; + if (psCurr == psDroid) + { + break; + } delete psCurr; } } @@ -530,15 +531,18 @@ bool removeDroidBase(DROID *psDel) if (psDel->psGroup) { //free all droids associated with this Transporter - DROID *psNext; - for (auto psCurr = psDel->psGroup->psList; psCurr != nullptr && psCurr != psDel; psCurr = psNext) + mutating_list_iterate(psDel->psGroup->psList, [psDel](DROID* psCurr) { - psNext = psCurr->psGrpNext; - + if (psCurr == psDel) + { + return IterationResult::BREAK_ITERATION; + } /* add droid to droid list then vanish it - hope this works! - GJ */ addDroid(psCurr, apsDroidLists); vanishDroid(psCurr); - } + + return IterationResult::CONTINUE_ITERATION; + }); } } @@ -2292,7 +2296,7 @@ UDWORD getNumDroidsForLevel(uint32_t player, UDWORD level) } if (isTransporter(psDroid)) { - for (DROID *psCurr = psDroid->psGroup->psList; psCurr != nullptr; psCurr = psCurr->psGrpNext) + for (const DROID *psCurr : psDroid->psGroup->psList) { if (psCurr != psDroid && getDroidLevel(psCurr) == level) { @@ -2708,7 +2712,7 @@ bool electronicDroid(const DROID *psDroid) if (psDroid->droidType == DROID_COMMAND && psDroid->psGroup && psDroid->psGroup->psCommander == psDroid) { // if a commander has EW units attached it is electronic - for (const DROID *psCurr = psDroid->psGroup->psList; psCurr; psCurr = psCurr->psGrpNext) + for (const DROID *psCurr : psDroid->psGroup->psList) { if (psDroid != psCurr && electronicDroid(psCurr)) { diff --git a/src/droiddef.h b/src/droiddef.h index 3923da1d8a8..4aaedbdef04 100644 --- a/src/droiddef.h +++ b/src/droiddef.h @@ -115,7 +115,6 @@ struct DROID : public BASE_OBJECT SWORD resistance; ///< used in Electronic Warfare // The group the droid belongs to DROID_GROUP *psGroup; - DROID *psGrpNext; STRUCTURE *psBaseStruct; ///< a structure that this droid might be associated with. For VTOLs this is the rearming pad // queued orders SDWORD listSize; ///< Gives the number of synchronised orders. Orders from listSize to the real end of the list may not affect game state. diff --git a/src/game.cpp b/src/game.cpp index 9f6d363ce79..1c438202512 100644 --- a/src/game.cpp +++ b/src/game.cpp @@ -5341,7 +5341,7 @@ static bool loadSaveDroidPointers(const WzString &pFileName, PerPlayerDroidLists } if (isTransporter(psDroid) && psDroid->psGroup != nullptr) // Check for droids in the transporter. { - for (DROID *psTrDroid = psDroid->psGroup->psList; psTrDroid != nullptr; psTrDroid = psTrDroid->psGrpNext) + for (DROID *psTrDroid : psDroid->psGroup->psList) { if (psTrDroid->id == id) { @@ -5485,7 +5485,7 @@ static void writeSaveObject(WzConfig &ini, const BASE_OBJECT *psObj) } } -static void writeSaveObjectJSON(nlohmann::json &jsonObj, BASE_OBJECT *psObj) +static void writeSaveObjectJSON(nlohmann::json &jsonObj, const BASE_OBJECT *psObj) { jsonObj["id"] = psObj->id; setPlayerJSON(jsonObj, psObj->player); @@ -5787,7 +5787,7 @@ static bool loadSaveDroid(const char *pFileName, PerPlayerDroidLists& ppsCurrent /* Writes the linked list of droids for each player to a file */ -static nlohmann::json writeDroid(DROID *psCurr, bool onMission, int &counter) +static nlohmann::json writeDroid(const DROID *psCurr, bool onMission, int &counter) { nlohmann::json droidObj = nlohmann::json::object(); droidObj["name"] = psCurr->aName; @@ -5918,7 +5918,7 @@ static bool writeDroidFile(const char *pFileName, const PerPlayerDroidLists& pps { if (psCurr->psGroup) { - for (DROID *psTrans = psCurr->psGroup->psList; psTrans != nullptr; psTrans = psTrans->psGrpNext) + for (const DROID *psTrans : psCurr->psGroup->psList) { if (psTrans != psCurr) { diff --git a/src/group.cpp b/src/group.cpp index 0121747016e..36bc3e6cd16 100644 --- a/src/group.cpp +++ b/src/group.cpp @@ -66,7 +66,6 @@ DROID_GROUP::DROID_GROUP() { type = GT_NORMAL; refCount = 0; - psList = nullptr; psCommander = nullptr; } @@ -114,7 +113,7 @@ void DROID_GROUP::add(DROID *psDroid) // if psDroid == NULL just increase the refcount don't add anything to the list if (psDroid != nullptr) { - if (psList && psDroid->player != psList->player) + if (!psList.empty() && psDroid->player != psList.front()->player) { ASSERT(false, "grpJoin: Cannot have more than one players droids in a group"); return; @@ -129,10 +128,9 @@ void DROID_GROUP::add(DROID *psDroid) if (isTransporter(psDroid)) { ASSERT_OR_RETURN(, (type == GT_NORMAL), "grpJoin: Cannot have two transporters in a group"); - ASSERT_OR_RETURN(, psList == nullptr, "Adding transporter to non-empty list."); + ASSERT_OR_RETURN(, psList.empty(), "Adding transporter to non-empty list."); type = GT_TRANSPORTER; - psDroid->psGrpNext = psList; - psList = psDroid; + psList.push_front(psDroid); } else if ((psDroid->droidType == DROID_COMMAND) && (type != GT_TRANSPORTER)) { @@ -142,8 +140,7 @@ void DROID_GROUP::add(DROID *psDroid) } else { - psDroid->psGrpNext = psList; - psList = psDroid; + psList.push_front(psDroid); } if (type == GT_COMMAND) @@ -158,8 +155,6 @@ void DROID_GROUP::add(DROID *psDroid) // remove a droid from a group void DROID_GROUP::remove(DROID *psDroid) { - DROID *psPrev, *psCurr; - ASSERT_OR_RETURN(, grpInitialized, "Group code not initialized yet"); if (psDroid != nullptr && psDroid->psGroup != this) @@ -178,34 +173,15 @@ void DROID_GROUP::remove(DROID *psDroid) // if psDroid == NULL just decrease the refcount don't remove anything from the list if (psDroid != nullptr) { - // update group list of droids and droids' psGrpNext + // update group list of droids if (psDroid->droidType != DROID_COMMAND || type != GT_COMMAND) { - psPrev = nullptr; - for (psCurr = psList; psCurr; psCurr = psCurr->psGrpNext) - { - if (psCurr == psDroid) - { - break; - } - psPrev = psCurr; - } - ASSERT(psCurr != nullptr, "grpLeave: droid not found"); - if (psCurr != nullptr) - { - if (psPrev) - { - psPrev->psGrpNext = psCurr->psGrpNext; - } - else - { - psList = psList->psGrpNext; - } - } + auto it = std::find(psList.begin(), psList.end(), psDroid); + ASSERT(it != psList.end(), "grpLeave: droid not found"); + psList.erase(it); } psDroid->psGroup = nullptr; - psDroid->psGrpNext = nullptr; // update group's type if ((psDroid->droidType == DROID_COMMAND) && (type == GT_COMMAND)) @@ -232,28 +208,16 @@ void DROID_GROUP::remove(DROID *psDroid) // count the members of a group unsigned int DROID_GROUP::getNumMembers() { - const DROID *psCurr; - unsigned int num; - ASSERT(grpInitialized, "Group code not initialized yet"); - - num = 0; - for (psCurr = psList; psCurr; psCurr = psCurr->psGrpNext) - { - num += 1; - } - - return num; + return psList.size(); } // Give a group of droids an order void DROID_GROUP::orderGroup(DROID_ORDER order) { - DROID *psCurr; - ASSERT(grpInitialized, "Group code not initialized yet"); - for (psCurr = psList; psCurr; psCurr = psCurr->psGrpNext) + for (DROID* psCurr : psList) { orderDroid(psCurr, order, ModeQueue); } @@ -262,12 +226,10 @@ void DROID_GROUP::orderGroup(DROID_ORDER order) // Give a group of droids an order (using a Location) void DROID_GROUP::orderGroup(DROID_ORDER order, UDWORD x, UDWORD y) { - DROID *psCurr; - ASSERT(grpInitialized, "Group code not initialized yet"); ASSERT_OR_RETURN(, validOrderForLoc(order), "orderGroup: Bad order"); - for (psCurr = psList; psCurr != nullptr; psCurr = psCurr->psGrpNext) + for (DROID* psCurr : psList) { orderDroidLoc(psCurr, order, x, y, bMultiMessages ? ModeQueue : ModeImmediate); } @@ -276,11 +238,9 @@ void DROID_GROUP::orderGroup(DROID_ORDER order, UDWORD x, UDWORD y) // Give a group of droids an order (using an Object) void DROID_GROUP::orderGroup(DROID_ORDER order, BASE_OBJECT *psObj) { - DROID *psCurr; - ASSERT_OR_RETURN(, validOrderForObj(order), "orderGroup: Bad order"); - for (psCurr = psList; psCurr != nullptr; psCurr = psCurr->psGrpNext) + for (DROID* psCurr : psList) { orderDroidObj(psCurr, order, (BASE_OBJECT *)psObj, bMultiMessages ? ModeQueue : ModeImmediate); } @@ -289,11 +249,9 @@ void DROID_GROUP::orderGroup(DROID_ORDER order, BASE_OBJECT *psObj) // Set the secondary state for a group of droids void DROID_GROUP::setSecondary(SECONDARY_ORDER sec, SECONDARY_STATE state) { - DROID *psCurr; - ASSERT(grpInitialized, "Group code not initialized yet"); - for (psCurr = psList; psCurr; psCurr = psCurr->psGrpNext) + for (DROID* psCurr : psList) { secondarySetState(psCurr, sec, state); } diff --git a/src/group.h b/src/group.h index 07c1bc8efa4..6f838b231ec 100644 --- a/src/group.h +++ b/src/group.h @@ -25,6 +25,7 @@ #define __INCLUDED_SRC_GROUP_H__ #include "orderdef.h" +#include "objmem.h" struct BASE_OBJECT; struct DROID; @@ -51,11 +52,11 @@ class DROID_GROUP void setSecondary(SECONDARY_ORDER sec, SECONDARY_STATE state); // set the secondary state for a group of droids - GROUP_TYPE type; // Type from the enum GROUP_TYPE above - SWORD refCount; // Number of objects in the group. Group is deleted if refCount<=0. Count number of droids+NULL pointers. - DROID *psList; // List of droids in the group - DROID *psCommander; // The command droid of a command group - int id; // unique group id + GROUP_TYPE type; // Type from the enum GROUP_TYPE above + SWORD refCount; // Number of objects in the group. Group is deleted if refCount<=0. Count number of droids+NULL pointers. + DroidList psList; // List of droids in the group + DROID *psCommander; // The command droid of a command group + int id; // unique group id }; // initialise the group system diff --git a/src/loop.cpp b/src/loop.cpp index 1d50b484044..bd1ebb7f80b 100644 --- a/src/loop.cpp +++ b/src/loop.cpp @@ -897,8 +897,6 @@ void adjustDroidCount(DROID *droid, int delta) { // Increase counts of droids in a transporter void droidCountsInTransporter(DROID *droid, int player) { - DROID *psDroid = nullptr; - if (!isTransporter(droid) || droid->psGroup == nullptr) { return; @@ -907,8 +905,12 @@ void droidCountsInTransporter(DROID *droid, int player) numTransporterDroids[player] += droid->psGroup->refCount - 1; // and count the units inside it... - for (psDroid = droid->psGroup->psList; psDroid != nullptr && psDroid != droid; psDroid = psDroid->psGrpNext) + for (DROID* psDroid : droid->psGroup->psList) { + if (psDroid == droid) + { + break; + } if (psDroid->droidType == DROID_CYBORG_CONSTRUCT || psDroid->droidType == DROID_CONSTRUCT) { numConstructorDroids[player] += 1; diff --git a/src/mission.cpp b/src/mission.cpp index 616ce8d7e08..eb6b5276d53 100644 --- a/src/mission.cpp +++ b/src/mission.cpp @@ -994,8 +994,6 @@ void restoreMissionLimboData() next - saves out the list of droids for the selected player*/ void saveCampaignData() { - DROID *psCurr, *psCurrNext; - debug(LOG_SAVE, "called"); ASSERT(selectedPlayer < MAX_PLAYERS, "selectedPlayer %" PRIu32 " exceeds MAX_PLAYERS", selectedPlayer); @@ -1014,9 +1012,12 @@ void saveCampaignData() // Empty the transporter into the mission list ASSERT_OR_RETURN(, psDroid->psGroup != nullptr, "Transporter does not have a group"); - for (psCurr = psDroid->psGroup->psList; psCurr != nullptr && psCurr != psDroid; psCurr = psCurrNext) + mutating_list_iterate(psDroid->psGroup->psList, [psDroid](DROID* psCurr) { - psCurrNext = psCurr->psGrpNext; + if (psCurr == psDroid) + { + return IterationResult::BREAK_ITERATION; + } // Remove it from the transporter group psDroid->psGroup->remove(psCurr); // Cam change add droid @@ -1024,7 +1025,9 @@ void saveCampaignData() psCurr->pos.y = INVALID_XY; // Add it back into current droid lists addDroid(psCurr, mission.apsDroidLists); - } + + return IterationResult::CONTINUE_ITERATION; + }); // Remove the transporter from the current list if (droidRemove(psDroid, apsDroidLists)) { @@ -1710,7 +1713,6 @@ static void missionResetDroids() goingHome = true when returning from an off World mission*/ void unloadTransporter(DROID *psTransporter, UDWORD x, UDWORD y, bool goingHome) { - DROID *psDroid, *psNext; PerPlayerDroidLists* ppCurrentList; UDWORD droidX, droidY; DROID_GROUP *psGroup; @@ -1729,9 +1731,12 @@ void unloadTransporter(DROID *psTransporter, UDWORD x, UDWORD y, bool goingHome) if (isTransporter(psTransporter)) { ASSERT(psTransporter->psGroup != nullptr, "psTransporter->psGroup is null??"); - for (psDroid = psTransporter->psGroup->psList; psDroid != nullptr && psDroid != psTransporter; psDroid = psNext) + for (DROID* psDroid : psTransporter->psGroup->psList) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + break; + } //add it back into current droid lists addDroid(psDroid, *ppCurrentList); @@ -1772,20 +1777,24 @@ void unloadTransporter(DROID *psTransporter, UDWORD x, UDWORD y, bool goingHome) transporterSetScriptCurrent(nullptr); /* remove droids from transporter group if not already transferred to script group */ - for (psDroid = psTransporter->psGroup->psList; psDroid != nullptr - && psDroid != psTransporter; psDroid = psNext) + mutating_list_iterate(psTransporter->psGroup->psList, [&psGroup, psTransporter](DROID* psDroid) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + return IterationResult::BREAK_ITERATION; + } // a commander needs to get it's group back if (psDroid->droidType == DROID_COMMAND) { psGroup = grpCreate(); psGroup->add(psDroid); clearCommandDroidFactory(psDroid); - continue; + return IterationResult::CONTINUE_ITERATION; } psTransporter->psGroup->remove(psDroid); - } + + return IterationResult::CONTINUE_ITERATION; + }); } // Don't do this in multiPlayer @@ -3061,22 +3070,25 @@ being flown to safety. The droids inside the Transporter are placed into the mission list for later use*/ void moveDroidsToSafety(DROID *psTransporter) { - DROID *psDroid, *psNext; - ASSERT_OR_RETURN(, isTransporter(psTransporter), "unit not a Transporter"); if (psTransporter->psGroup != nullptr) { //move droids out of Transporter into mission list - for (psDroid = psTransporter->psGroup->psList; psDroid != nullptr && psDroid != psTransporter; psDroid = psNext) + mutating_list_iterate(psTransporter->psGroup->psList, [psTransporter](DROID* psDroid) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + return IterationResult::BREAK_ITERATION; + } psTransporter->psGroup->remove(psDroid); //cam change add droid psDroid->pos.x = INVALID_XY; psDroid->pos.y = INVALID_XY; addDroid(psDroid, mission.apsDroidLists); - } + + return IterationResult::CONTINUE_ITERATION; + }); } //move the transporter into the mission list also @@ -3216,7 +3228,6 @@ std::vector readCampaignFiles() mission ends. bOffWorld is true if the Mission is currently offWorld*/ void emptyTransporters(bool bOffWorld) { - DROID* psNext; ASSERT_OR_RETURN(, selectedPlayer < MAX_PLAYERS, "selectedPlayer %" PRIu32 " >= MAX_PLAYERS", selectedPlayer); //see if there are any Transporters in the world @@ -3234,29 +3245,37 @@ void emptyTransporters(bool bOffWorld) and processMission() will assign them a location etc */ if (bOffWorld) { - for (DROID* psDroid = psTransporter->psGroup->psList; psDroid != nullptr - && psDroid != psTransporter; psDroid = psNext) + mutating_list_iterate(psTransporter->psGroup->psList, [psTransporter](DROID* psDroid) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + return IterationResult::BREAK_ITERATION; + } //take it out of the Transporter group psTransporter->psGroup->remove(psDroid); //add it back into current droid lists addDroid(psDroid, apsDroidLists); - } + + return IterationResult::CONTINUE_ITERATION; + }); } /* we're not offWorld so add to mission.apsDroidList to be processed by the endMission function */ else { - for (DROID* psDroid = psTransporter->psGroup->psList; psDroid != nullptr - && psDroid != psTransporter; psDroid = psNext) + mutating_list_iterate(psTransporter->psGroup->psList, [psTransporter](DROID* psDroid) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + return IterationResult::BREAK_ITERATION; + } //take it out of the Transporter group psTransporter->psGroup->remove(psDroid); //add it back into current droid lists addDroid(psDroid, mission.apsDroidLists); - } + + return IterationResult::CONTINUE_ITERATION; + }); } //now kill off the Transporter vanishDroid(psTransporter); @@ -3278,15 +3297,19 @@ void emptyTransporters(bool bOffWorld) if (isTransporter(psTransporter)) { //for each droid within the transporter... - for (DROID* psDroid = psTransporter->psGroup->psList; psDroid != nullptr - && psDroid != psTransporter; psDroid = psNext) + mutating_list_iterate(psTransporter->psGroup->psList, [psTransporter](DROID* psDroid) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + return IterationResult::BREAK_ITERATION; + } //take it out of the Transporter group psTransporter->psGroup->remove(psDroid); //add it back into mission droid lists addDroid(psDroid, mission.apsDroidLists); - } + + return IterationResult::CONTINUE_ITERATION; + }); } //don't need to destroy the transporter here - it is dealt with by the endMission process transIt = transItNext; diff --git a/src/multibot.cpp b/src/multibot.cpp index 99f6c181bba..a115fc4f651 100644 --- a/src/multibot.cpp +++ b/src/multibot.cpp @@ -243,7 +243,6 @@ bool sendDroidDisembark(DROID const *psTransporter, DROID const *psDroid) bool recvDroidDisEmbark(NETQUEUE queue) { DROID *psFoundDroid = nullptr, *psTransporterDroid = nullptr; - DROID *psCheckDroid = nullptr; NETbeginDecode(queue, GAME_DROIDDISEMBARK); { @@ -270,17 +269,18 @@ bool recvDroidDisEmbark(NETQUEUE queue) return false; } // we need to find the droid *in* the transporter - psCheckDroid = (psTransporterDroid->psGroup != nullptr) ? psTransporterDroid->psGroup->psList : nullptr; - while (psCheckDroid) + if (psTransporterDroid->psGroup) { - // is this the one we want? - if (psCheckDroid->id == droidID) + const auto& groupList = psTransporterDroid->psGroup->psList; + auto it = std::find_if(groupList.begin(), groupList.end(), + [droidID](DROID* d) + { + return d->id == droidID; + }); + if (it != groupList.end()) { - psFoundDroid = psCheckDroid; - break; + psFoundDroid = *it; } - // not found, so check next one in *group* - psCheckDroid = psCheckDroid->psGrpNext; } // don't continue if we couldn't find it. if (!psFoundDroid) diff --git a/src/objmem.cpp b/src/objmem.cpp index 1f645bc170c..7e49e96c854 100644 --- a/src/objmem.cpp +++ b/src/objmem.cpp @@ -708,10 +708,10 @@ static BASE_OBJECT* getBaseObjFromDroidId(const DroidList& list, unsigned id) return psObj; } // if transporter check any droids in the grp - if ((psObj->type == OBJ_DROID) && isTransporter((DROID*)psObj)) + if ((psObj->type == OBJ_DROID) && isTransporter(psObj)) { - ASSERT_OR_RETURN(nullptr, ((DROID*)psObj)->psGroup != nullptr, "Transporter has null group?"); - for (DROID* psTrans = ((DROID*)psObj)->psGroup->psList; psTrans != nullptr; psTrans = psTrans->psGrpNext) + ASSERT_OR_RETURN(nullptr, psObj->psGroup != nullptr, "Transporter has null group?"); + for (DROID* psTrans : psObj->psGroup->psList) { if (psTrans->id == id) { @@ -915,14 +915,9 @@ void objCount(int *droids, int *structures, int *features) for (const DROID *psDroid : apsDroidLists[i]) { (*droids)++; - if (isTransporter(psDroid) && psDroid->psGroup && psDroid->psGroup->psList) + if (isTransporter(psDroid) && psDroid->psGroup && !psDroid->psGroup->psList.empty()) { - const DROID *psTrans = psDroid->psGroup->psList; - - for (psTrans = psTrans->psGrpNext; psTrans != nullptr; psTrans = psTrans->psGrpNext) - { - (*droids)++; - } + *droids += psDroid->psGroup->psList.size(); } } diff --git a/src/objmem.h b/src/objmem.h index a1a61f55521..f452e068925 100644 --- a/src/objmem.h +++ b/src/objmem.h @@ -28,6 +28,7 @@ #include #include +#include /* The lists of objects allocated */ template @@ -153,4 +154,30 @@ void objCount(int *droids, int *structures, int *features); void checkFactoryFlags(); #endif +enum class IterationResult +{ + BREAK_ITERATION, + CONTINUE_ITERATION +}; + +// Common iteration helper for lists of game objects +// with an ability to execute loop body handlers which can +// possibly invalidate the any iterator in the range `[begin(), currentIterator]`. +template +void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHandler handler) +{ + typename std::remove_reference_t::iterator it = list.begin(), itNext; + while (it != list.end()) + { + itNext = std::next(it); + // Can possibly invalidate `it` and anything before it. + const auto res = handler(*it); + if (res == IterationResult::BREAK_ITERATION) + { + break; + } + it = itNext; + } +} + #endif // __INCLUDED_SRC_OBJMEM_H__ diff --git a/src/order.cpp b/src/order.cpp index b46530c49f7..812ee950e6b 100644 --- a/src/order.cpp +++ b/src/order.cpp @@ -1249,7 +1249,7 @@ static void orderCmdGroupBase(DROID_GROUP *psGroup, DROID_ORDER_DATA *psData) // picking up an artifact - only need to send one unit DROID *psChosen = nullptr; int mindist = SDWORD_MAX; - for (DROID *psCurr = psGroup->psList; psCurr; psCurr = psCurr->psGrpNext) + for (DROID* psCurr : psGroup->psList) { if (psCurr->order.type == DORDER_RTR || psCurr->order.type == DORDER_RTB || psCurr->order.type == DORDER_RTR_SPECIFIED) { @@ -1272,7 +1272,7 @@ static void orderCmdGroupBase(DROID_GROUP *psGroup, DROID_ORDER_DATA *psData) else { const bool isAttackOrder = psData->type == DORDER_ATTACKTARGET || psData->type == DORDER_ATTACK; - for (DROID *psCurr = psGroup->psList; psCurr; psCurr = psCurr->psGrpNext) + for (DROID* psCurr : psGroup->psList) { syncDebug("command %d", psCurr->id); if (!orderState(psCurr, DORDER_RTR)) // if you change this, youll need to change sendcmdgroup() @@ -1427,7 +1427,7 @@ void orderDroidBase(DROID *psDroid, DROID_ORDER_DATA *psOrder) // the commander doesn't have to pick up artifacts, one // of his units will do it for him (if there are any in his group). if ((psOrder->type == DORDER_RECOVER) && - (psDroid->psGroup->psList != nullptr)) + (!psDroid->psGroup->psList.empty())) { psOrder->type = DORDER_NONE; } @@ -3479,7 +3479,7 @@ bool secondarySetState(DROID *psDroid, SECONDARY_ORDER sec, SECONDARY_STATE Stat UDWORD CurrState, factType, prodType; SDWORD factoryInc; bool retVal; - DROID *psTransport, *psCurr, *psNext; + DROID *psTransport; DROID_ORDER order; CurrState = psDroid->secondaryOrder; @@ -3758,12 +3758,13 @@ bool secondarySetState(DROID *psDroid, SECONDARY_ORDER sec, SECONDARY_STATE Stat if (psDroid->droidType == DROID_COMMAND) { // remove all the units from the commanders group - for (psCurr = psDroid->psGroup->psList; psCurr; psCurr = psNext) + mutating_list_iterate(psDroid->psGroup->psList, [](DROID* psCurr) { - psNext = psCurr->psGrpNext; psCurr->psGroup->remove(psCurr); orderDroid(psCurr, DORDER_STOP, ModeImmediate); - } + + return IterationResult::CONTINUE_ITERATION; + }); } else if (psDroid->psGroup->type == GT_COMMAND) { diff --git a/src/research.cpp b/src/research.cpp index a63ab6c75d2..ab132b91516 100644 --- a/src/research.cpp +++ b/src/research.cpp @@ -1596,12 +1596,9 @@ void replaceDroidComponent(DroidList& pList, UDWORD oldType, UDWORD oldCompInc, void replaceTransDroidComponents(DROID *psTransporter, UDWORD oldType, UDWORD oldCompInc, UDWORD newCompInc) { - DROID *psCurr; - ASSERT(isTransporter(psTransporter), "invalid unit type"); - for (psCurr = psTransporter->psGroup->psList; psCurr != nullptr; psCurr = - psCurr->psGrpNext) + for (DROID* psCurr : psTransporter->psGroup->psList) { if (psCurr != psTransporter) { diff --git a/src/scores.cpp b/src/scores.cpp index e2a9a6eb480..c6340b7309e 100644 --- a/src/scores.cpp +++ b/src/scores.cpp @@ -312,7 +312,7 @@ END_GAME_STATS_DATA collectEndGameStatsData() { if (isTransporter(psDroid)) { - for (DROID *psCurr = psDroid->psGroup->psList; psCurr != nullptr; psCurr = psCurr->psGrpNext) + for (DROID *psCurr : psDroid->psGroup->psList) { if (psCurr != psDroid) { diff --git a/src/transporter.cpp b/src/transporter.cpp index 9102f78711a..cb18fc9c212 100644 --- a/src/transporter.cpp +++ b/src/transporter.cpp @@ -325,7 +325,6 @@ bool intAddTransporterContents() bool intAddTransporterLaunch(DROID *psDroid) { UDWORD capacity; - DROID *psCurr, *psNext; if (bMultiPlayer) { @@ -382,9 +381,8 @@ bool intAddTransporterLaunch(DROID *psDroid) if (psCurrTransporter && psCurrTransporter->psGroup) { capacity = TRANSPORTER_CAPACITY; - for (psCurr = psCurrTransporter->psGroup->psList; psCurr != nullptr; psCurr = psNext) + for (const DROID* psCurr : psCurrTransporter->psGroup->psList) { - psNext = psCurr->psGrpNext; if (psCurr != psCurrTransporter) { capacity -= transporterSpaceRequired(psCurr); @@ -507,8 +505,12 @@ bool intAddTransContentsForm() ASSERT_OR_RETURN(false, psCurrTransporter->psGroup != nullptr, "Null transporter group"); - for (DROID *psDroid = psCurrTransporter->psGroup->psList; psDroid != nullptr && psDroid != psCurrTransporter; psDroid = psDroid->psGrpNext) + for (DROID* psDroid : psCurrTransporter->psGroup->psList) { + if (psDroid == psCurrTransporter) + { + break; + } if (psDroid->selected) { continue; // Droid is queued to be ejected from the transport, so don't display it. @@ -652,7 +654,6 @@ up different amount depending on their body size - currently all are set to one! int calcRemainingCapacity(const DROID *psTransporter) { int capacity = TRANSPORTER_CAPACITY; - const DROID *psDroid, *psNext; // If it's dead then just return 0. if (isDead((const BASE_OBJECT *)psTransporter)) @@ -666,9 +667,12 @@ int calcRemainingCapacity(const DROID *psTransporter) return 0; } - for (psDroid = psTransporter->psGroup->psList; psDroid != nullptr && psDroid != psTransporter; psDroid = psNext) + for (DROID* psDroid : psTransporter->psGroup->psList) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + break; + } const int space = transporterSpaceRequired(psDroid); ASSERT(space > 0, "Invalid space required for %s", objInfo(psDroid)); capacity -= space; @@ -689,8 +693,8 @@ bool transporterIsEmpty(const DROID *psTransporter) // Assume dead droids and non-transporter droids to be empty return (isDead((const BASE_OBJECT *)psTransporter) || !isTransporter(psTransporter) - || psTransporter->psGroup->psList == nullptr - || psTransporter->psGroup->psList == psTransporter); + || psTransporter->psGroup->psList.empty() + || psTransporter->psGroup->psList.front() == psTransporter); } static void intSetTransCapacityLabel(W_LABEL &Label) @@ -732,22 +736,22 @@ void intProcessTransporter(UDWORD id) if (psCurrTransporter != nullptr && !transporterFlying(psCurrTransporter)) { unsigned currID = IDTRANS_CONTSTART; - DROID *psDroid; - for (psDroid = psCurrTransporter->psGroup->psList; psDroid != nullptr && psDroid != psCurrTransporter; psDroid = psDroid->psGrpNext) + const auto& groupList = psCurrTransporter->psGroup->psList; + const auto psDroidIt = std::find_if(groupList.begin(), groupList.end(), [&currID, id](DROID* psDroid) { - if (psDroid->selected) + if (psDroid == psCurrTransporter) { - continue; // Already scheduled this droid for removal. + return false; } - if (currID == id) + if (psDroid->selected) { - break; + return false; // Already scheduled this droid for removal. } - currID++; - } - if (psDroid != nullptr) + return currID++ == id; + }); + if (psDroidIt != groupList.end()) { - transporterRemoveDroid(psCurrTransporter, psDroid, ModeQueue); + transporterRemoveDroid(psCurrTransporter, *psDroidIt, ModeQueue); } /*refresh the Contents list */ intAddTransporterContents(); @@ -1080,7 +1084,6 @@ void transporterAddDroid(DROID *psTransporter, DROID *psDroidToAdd) /*check to see if the droid can fit on the Transporter - return true if fits*/ bool checkTransporterSpace(DROID const *psTransporter, DROID const *psAssigned, bool mayFlash) { - DROID *psDroid, *psNext; UDWORD capacity; ASSERT_OR_RETURN(false, psTransporter != nullptr, "Invalid droid pointer"); @@ -1090,9 +1093,12 @@ bool checkTransporterSpace(DROID const *psTransporter, DROID const *psAssigned, //work out how much space is currently left capacity = TRANSPORTER_CAPACITY; - for (psDroid = psTransporter->psGroup->psList; psDroid != nullptr && psDroid != psTransporter; psDroid = psNext) + for (DROID* psDroid : psTransporter->psGroup->psList) { - psNext = psDroid->psGrpNext; + if (psDroid == psTransporter) + { + break; + } capacity -= transporterSpaceRequired(psDroid); } if (capacity >= transporterSpaceRequired(psAssigned)) diff --git a/src/wzapi.cpp b/src/wzapi.cpp index 15b99aa5edd..66b8f4d26fb 100644 --- a/src/wzapi.cpp +++ b/src/wzapi.cpp @@ -2281,8 +2281,7 @@ std::vector wzapi::enumCargo(WZAPI_PARAMS(const DROID *psDroid)) std::vector result; SCRIPT_ASSERT({}, context, psDroid->psGroup, "Droid is not assigned to a group"); result.reserve(psDroid->psGroup->getNumMembers()); - int i = 0; - for (DROID *psCurr = psDroid->psGroup->psList; psCurr; psCurr = psCurr->psGrpNext, i++) + for (const DROID *psCurr : psDroid->psGroup->psList) { if (psDroid != psCurr) {