From de6a149cc7319c824d05d2435aaf59f3a99e4995 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 3 Mar 2024 12:36:41 +0300 Subject: [PATCH] Switch `DROID` to use `PagedEntityContainer` as backing storage 1. Split the droid storage into pages containing 256 droids 2. Disable slot reuse to guard against memory-related issues when some object pointers won't get updated properly, e.g. when transitioning between the base and offworld missions. Signed-off-by: Pavel Solodovnikov --- src/component.h | 2 +- src/droid.cpp | 87 ++++++++++++++++++++++++++--------------------- src/droid.h | 7 ++++ src/mechanics.cpp | 2 +- src/objmem.cpp | 38 +++++++++++++++++---- src/objmem.h | 4 +++ src/wzapi.cpp | 1 + 7 files changed, 94 insertions(+), 47 deletions(-) diff --git a/src/component.h b/src/component.h index 6280187759b..65bf2906a58 100644 --- a/src/component.h +++ b/src/component.h @@ -83,7 +83,7 @@ void drawMuzzleFlash(WEAPON sWeap, iIMDShape *weaponImd, iIMDShape *flashImd, PI #define PART_IMD(STATS,DROID,COMPONENT,PLAYER) (STATS[DROID->asBits[COMPONENT]].pIMD) /* Get the chassis imd */ -#define BODY_IMD(DROID,PLAYER) (DROID->getBodyStats()->pIMD) +#define BODY_IMD(DROID,PLAYER) ((DROID)->getBodyStats()->pIMD) /* Get the brain imd - NOTE: Unused!*/ #define BRAIN_IMD(DROID,PLAYER) (DROID->getBrainStats()->pIMD) /* Get the weapon imd */ diff --git a/src/droid.cpp b/src/droid.cpp index 237868c1b14..c0e3f64269a 100644 --- a/src/droid.cpp +++ b/src/droid.cpp @@ -444,14 +444,17 @@ DROID::~DROID() if (psDroid->psGroup) { //free all droids associated with this Transporter - mutating_list_iterate(psDroid->psGroup->psList, [psDroid](DROID* psCurr) + auto& droidContainer = GlobalDroidContainer(); + mutating_list_iterate(psDroid->psGroup->psList, [psDroid, &droidContainer](DROID* psCurr) { if (psCurr == psDroid) { return IterationResult::BREAK_ITERATION; } // This will cause each droid to self-remove from `psGroup->psList`. - delete psCurr; + auto it = droidContainer.find(*psCurr); + ASSERT(it != droidContainer.end(), "Droid not found in the global container!"); + droidContainer.erase(it); return IterationResult::CONTINUE_ITERATION; }); } @@ -1624,90 +1627,90 @@ DROID *reallyBuildDroid(const DROID_TEMPLATE *pTemplate, Position pos, UDWORD pl ASSERT_OR_RETURN(nullptr, player < MAX_PLAYERS, "Invalid player: %" PRIu32 "", player); - DROID *psDroid = new DROID(id, player); - droidSetName(psDroid, getLocalizedStatsName(pTemplate)); + DROID& droid = GlobalDroidContainer().emplace(id, player); + droidSetName(&droid, getLocalizedStatsName(pTemplate)); // Set the droids type - psDroid->droidType = droidTemplateType(pTemplate); // Is set again later to the same thing, in droidSetBits. - psDroid->pos = pos; - psDroid->rot = rot; + droid.droidType = droidTemplateType(pTemplate); // Is set again later to the same thing, in droidSetBits. + droid.pos = pos; + droid.rot = rot; //don't worry if not on homebase cos not being drawn yet if (!onMission) { //set droid height - psDroid->pos.z = map_Height(psDroid->pos.x, psDroid->pos.y); + droid.pos.z = map_Height(droid.pos.x, droid.pos.y); } - if (psDroid->isTransporter() || psDroid->droidType == DROID_COMMAND) + if (droid.isTransporter() || droid.droidType == DROID_COMMAND) { DROID_GROUP *psGrp = grpCreate(); - psGrp->add(psDroid); + psGrp->add(&droid); } // find the highest stored experience // Unless game time is stopped, then we're hopefully loading a game and // don't want to use up recycled experience for the droids we just loaded. if (!gameTimeIsStopped() && - (psDroid->droidType != DROID_CONSTRUCT) && - (psDroid->droidType != DROID_CYBORG_CONSTRUCT) && - (psDroid->droidType != DROID_REPAIR) && - (psDroid->droidType != DROID_CYBORG_REPAIR) && - !psDroid->isTransporter() && - !recycled_experience[psDroid->player].empty()) + (droid.droidType != DROID_CONSTRUCT) && + (droid.droidType != DROID_CYBORG_CONSTRUCT) && + (droid.droidType != DROID_REPAIR) && + (droid.droidType != DROID_CYBORG_REPAIR) && + !droid.isTransporter() && + !recycled_experience[droid.player].empty()) { - psDroid->experience = recycled_experience[psDroid->player].top(); - recycled_experience[psDroid->player].pop(); + droid.experience = recycled_experience[droid.player].top(); + recycled_experience[droid.player].pop(); } else { - psDroid->experience = 0; + droid.experience = 0; } - psDroid->kills = 0; + droid.kills = 0; - droidSetBits(pTemplate, psDroid); + droidSetBits(pTemplate, &droid); //calculate the droids total weight - psDroid->weight = calcDroidWeight(pTemplate); + droid.weight = calcDroidWeight(pTemplate); // Initialise the movement stuff - psDroid->baseSpeed = calcDroidBaseSpeed(pTemplate, psDroid->weight, (UBYTE)player); + droid.baseSpeed = calcDroidBaseSpeed(pTemplate, droid.weight, (UBYTE)player); - initDroidMovement(psDroid); + initDroidMovement(&droid); //allocate 'easy-access' data! - psDroid->body = calcDroidBaseBody(psDroid); // includes upgrades - ASSERT(psDroid->body > 0, "Invalid number of hitpoints"); - psDroid->originalBody = psDroid->body; + droid.body = calcDroidBaseBody(&droid); // includes upgrades + ASSERT(droid.body > 0, "Invalid number of hitpoints"); + droid.originalBody = droid.body; /* Set droid's initial illumination */ - psDroid->sDisplay.imd = BODY_IMD(psDroid, psDroid->player); + droid.sDisplay.imd = BODY_IMD(&droid, droid.player); //don't worry if not on homebase cos not being drawn yet if (!onMission) { /* People always stand upright */ - if (psDroid->droidType != DROID_PERSON) + if (droid.droidType != DROID_PERSON) { - updateDroidOrientation(psDroid); + updateDroidOrientation(&droid); } - visTilesUpdate(psDroid); + visTilesUpdate(&droid); } /* transporter-specific stuff */ - if (psDroid->isTransporter()) + if (droid.isTransporter()) { //add transporter launch button if selected player and not a reinforcable situation if (player == selectedPlayer && !missionCanReEnforce()) { - (void)intAddTransporterLaunch(psDroid); + (void)intAddTransporterLaunch(&droid); } //set droid height to be above the terrain - psDroid->pos.z += TRANSPORTER_HOVER_HEIGHT; + droid.pos.z += TRANSPORTER_HOVER_HEIGHT; /* reset halt secondary order from guard to hold */ - secondarySetState(psDroid, DSO_HALTTYPE, DSS_HALT_HOLD); + secondarySetState(&droid, DSO_HALTTYPE, DSS_HALT_HOLD); } if (player == selectedPlayer) @@ -1716,12 +1719,12 @@ DROID *reallyBuildDroid(const DROID_TEMPLATE *pTemplate, Position pos, UDWORD pl } // Avoid droid appearing to jump or turn on spawn. - psDroid->prevSpacetime.pos = psDroid->pos; - psDroid->prevSpacetime.rot = psDroid->rot; + droid.prevSpacetime.pos = droid.pos; + droid.prevSpacetime.rot = droid.rot; - debug(LOG_LIFE, "created droid for player %d, droid = %p, id=%d (%s): position: x(%d)y(%d)z(%d)", player, static_cast(psDroid), (int)psDroid->id, psDroid->aName, psDroid->pos.x, psDroid->pos.y, psDroid->pos.z); + debug(LOG_LIFE, "created droid for player %d, droid = %p, id=%d (%s): position: x(%d)y(%d)z(%d)", player, static_cast(&droid), (int)droid.id, droid.aName, droid.pos.x, droid.pos.y, droid.pos.z); - return psDroid; + return &droid; } DROID *reallyBuildDroid(const DROID_TEMPLATE *pTemplate, Position pos, UDWORD player, bool onMission, Rotation rot) @@ -3598,3 +3601,9 @@ CONSTRUCT_STATS* DROID::getConstructStats() const { return &asConstructStats[asBits[COMP_CONSTRUCT]]; } + +DroidContainer& GlobalDroidContainer() +{ + static DroidContainer instance; + return instance; +} diff --git a/src/droid.h b/src/droid.h index 1f1156dd63a..194ef03d6a3 100644 --- a/src/droid.h +++ b/src/droid.h @@ -24,6 +24,7 @@ #ifndef __INCLUDED_SRC_DROID_H__ #define __INCLUDED_SRC_DROID_H__ +#include "lib/framework/paged_entity_container.h" #include "lib/framework/string_ext.h" #include "lib/gamelib/gtime.h" @@ -425,4 +426,10 @@ static inline DROID const *castDroid(SIMPLE_OBJECT const *psObject) */ void droidWasFullyRepaired(DROID *psDroid, const REPAIR_FACILITY *psRepairFac); +// Split the droid storage into pages containing 256 droids, disable slot reuse +// to guard against memory-related issues when some object pointers won't get +// updated properly, e.g. when transitioning between the base and offworld missions. +using DroidContainer = PagedEntityContainer; +DroidContainer& GlobalDroidContainer(); + #endif // __INCLUDED_SRC_DROID_H__ diff --git a/src/mechanics.cpp b/src/mechanics.cpp index 35b7f0aa3d2..79c76f65f50 100644 --- a/src/mechanics.cpp +++ b/src/mechanics.cpp @@ -39,7 +39,7 @@ bool mechanicsShutdown() { for (BASE_OBJECT* psObj : psDestroyedObj) { - delete psObj; + objmemDestroy(psObj); } psDestroyedObj.clear(); diff --git a/src/objmem.cpp b/src/objmem.cpp index e54ddc598d5..22bd0bf3d18 100644 --- a/src/objmem.cpp +++ b/src/objmem.cpp @@ -201,7 +201,7 @@ static bool checkReferences(BASE_OBJECT *psVictim) /* Remove an object from the destroyed list, finally freeing its memory * Hopefully by this time, no pointers still refer to it! */ -static bool objmemDestroy(BASE_OBJECT *psObj) +bool objmemDestroy(BASE_OBJECT *psObj) { switch (psObj->type) { @@ -224,8 +224,19 @@ static bool objmemDestroy(BASE_OBJECT *psObj) { return false; } - debug(LOG_MEMORY, "BASE_OBJECT* 0x%p is freed.", static_cast(psObj)); - delete psObj; + // Droids are managed by a separate droid container. + if (psObj->type == OBJ_DROID) + { + auto& droidContainer = GlobalDroidContainer(); + auto it = droidContainer.find(*static_cast(psObj)); + ASSERT(it != droidContainer.end(), "Droid not found in the global container!"); + droidContainer.erase(it); + } + else + { + delete psObj; + } + debug(LOG_MEMORY, "BASE_OBJECT* is freed."); return true; } @@ -439,10 +450,25 @@ void killDroid(DROID *psDel) destroyObject(apsDroidLists, psDel); } +static void freeAllDroidsImpl(PerPlayerDroidLists& droidLists) +{ + auto& droidContainer = GlobalDroidContainer(); + for (auto& list : droidLists) + { + for (DROID* d : list) + { + auto it = droidContainer.find(*d); + ASSERT(it != droidContainer.end(), "Droid not found in the global container!"); + droidContainer.erase(it); + } + list.clear(); + } +} + /* Remove all droids */ void freeAllDroids() { - releaseAllObjectsInList(apsDroidLists); + freeAllDroidsImpl(apsDroidLists); } /*Remove a single Droid from a list*/ @@ -475,13 +501,13 @@ void removeDroid(DROID* psDroidToRemove, PerPlayerDroidLists& pList) /*Removes all droids that may be stored in the mission lists*/ void freeAllMissionDroids() { - releaseAllObjectsInList(mission.apsDroidLists); + freeAllDroidsImpl(mission.apsDroidLists); } /*Removes all droids that may be stored in the limbo lists*/ void freeAllLimboDroids() { - releaseAllObjectsInList(apsLimboDroids); + freeAllDroidsImpl(apsLimboDroids); } /************************** STRUCTURE *******************************/ diff --git a/src/objmem.h b/src/objmem.h index 3d3c4122d04..924dec133d4 100644 --- a/src/objmem.h +++ b/src/objmem.h @@ -74,6 +74,10 @@ void objmemShutdown(); /* General housekeeping for the object system */ void objmemUpdate(); +/* Remove an object from the destroyed list, finally freeing its memory + * Hopefully by this time, no pointers still refer to it! */ +bool objmemDestroy(BASE_OBJECT* psObj); + /// Generates a new, (hopefully) unique object id. uint32_t generateNewObjectId(); /// Generates a new, (hopefully) unique object id, which all clients agree on. diff --git a/src/wzapi.cpp b/src/wzapi.cpp index f15ab9bbd14..7b2fbccf307 100644 --- a/src/wzapi.cpp +++ b/src/wzapi.cpp @@ -2158,6 +2158,7 @@ std::unique_ptr wzapi::getDroidProduction(WZAPI_PARAMS(const STRUCT { return nullptr; } + // Since it's not intended to be used anywhere, don't put it in the global droid storage. DROID *psDroid = new DROID(0, player); psDroid->pos = psStruct->pos; psDroid->rot = psStruct->rot;