Switch droids to use PagedEntityContainer<DROID> as backing storage

lib/framework/paged_entity_container.h

@@ -42,7 +42,8 @@
 /// As noted above, the container allocates memory in fixed-size
 /// continuous chunks, or pages, hence the name.
 ///
-/// Currently, each page is set to hold exactly 1024 elements.
+/// Each page is set to hold exactly `MaxElementsPerPage` elements,
+/// which is 1024 by default.
 ///
 /// Also, each element is equipped with some additional metadata,
 /// which allows the container to reuse the same memory (also called "slots")
@@ -97,14 +98,15 @@
 /// * https://github.com/Masstronaut/slot_array/blob/master/slot_map.hpp
 /// </summary>
 /// <typeparam name="T">Entity type. Should be a complete type.</typeparam>
-template <typename T>
+/// <typeparam name="MaxElementsPerPage">The fixed number of elements each page may hold.</typeparam>
+/// <typeparam name="ReuseSlots">If `false`, slots are one-shot and set to expire after single use.</typeparam>
+template <typename T, size_t MaxElementsPerPage = 1024, bool ReuseSlots = true>
 class PagedEntityContainer
 {
 	using SlotIndexType = size_t;
 
-	static constexpr size_t MAX_ELEMENTS_PER_PAGE = 1024;
 	// Default initial capacity is exactly 1 page.
-	static constexpr size_t DEFAULT_INITIAL_CAPACITY = 1 * MAX_ELEMENTS_PER_PAGE;
+	static constexpr size_t DEFAULT_INITIAL_CAPACITY = 1 * MaxElementsPerPage;
 
 	static constexpr SlotIndexType INVALID_SLOT_IDX = std::numeric_limits<SlotIndexType>::max();
 	static constexpr size_t INVALID_PAGE_IDX = std::numeric_limits<size_t>::max();
@@ -149,6 +151,11 @@ class PagedEntityContainer
 			return _isAlive;
 		}
 
+		void invalidate()
+		{
+			_generation = INVALID_GENERATION;
+		}
+
 		void set_dead()
 		{
 			_isAlive = false;
@@ -174,7 +181,7 @@ class PagedEntityContainer
 	/// This class holds the actual contiguous storage and metadata for the elements,
 	/// as well as the queue for recycled indices.
 	///
-	/// Always allocates storage for exactly `MAX_ELEMENTS_PER_PAGE` elements.
+	/// Always allocates storage for exactly `MaxElementsPerPage` elements.
 	/// </summary>
 	class Page
 	{
@@ -195,17 +202,17 @@ class PagedEntityContainer
 
 		bool is_full() const
 		{
-			return _currentSize + _expiredSlotsCount == MAX_ELEMENTS_PER_PAGE;
+			return _currentSize + _expiredSlotsCount == MaxElementsPerPage;
 		}
 
 		void allocate_storage()
 		{
 			assert(_storage == nullptr);
 			assert(_slotMetadata == nullptr);
 
-			// Allocate storage for MAX_ELEMENTS_PER_PAGE elements.
-			_storage = std::make_unique<AlignedElementStorage[]>(MAX_ELEMENTS_PER_PAGE);
-			_slotMetadata = std::make_unique<SlotMetadata[]>(MAX_ELEMENTS_PER_PAGE);
+			// Allocate storage for `MaxElementsPerPage` elements.
+			_storage = std::make_unique<AlignedElementStorage[]>(MaxElementsPerPage);
+			_slotMetadata = std::make_unique<SlotMetadata[]>(MaxElementsPerPage);
 		}
 
 		bool has_recycled_indices() const
@@ -274,7 +281,7 @@ class PagedEntityContainer
 
 		bool is_expired() const
 		{
-			return _expiredSlotsCount == MAX_ELEMENTS_PER_PAGE;
+			return _expiredSlotsCount == MaxElementsPerPage;
 		}
 
 		// Reset generations to least possible valid value for all slots,
@@ -283,7 +290,7 @@ class PagedEntityContainer
 		{
 			auto* meta = slotMetadata();
 			assert(meta != nullptr);
-			for (size_t i = 0; i < MAX_ELEMENTS_PER_PAGE; ++i)
+			for (size_t i = 0; i < MaxElementsPerPage; ++i)
 			{
 				auto& slot = meta[i];
 				slot.reset_generation();
@@ -342,7 +349,7 @@ class PagedEntityContainer
 		{
 			return;
 		}
-		size_t needed_nr_of_pages = (capacity / MAX_ELEMENTS_PER_PAGE) - _pages.size();
+		size_t needed_nr_of_pages = (capacity / MaxElementsPerPage) - _pages.size();
 		while (needed_nr_of_pages-- != 0)
 		{
 			allocate_new_page();
@@ -354,7 +361,7 @@ class PagedEntityContainer
 		Page newPage;
 		newPage.allocate_storage();
 		_pages.emplace_back(std::move(newPage));
-		_capacity += MAX_ELEMENTS_PER_PAGE;
+		_capacity += MaxElementsPerPage;
 	}
 
 	template <typename... Args>
@@ -398,8 +405,11 @@ class PagedEntityContainer
 		{
 			return;
 		}
-		// Advance slot generation number.
-		slotMetadata.advance_generation();
+
+		// Either advance slot generation number or invalidate the slot right away,
+		// the behavior depends on whether we reuse the slots or not.
+		advance_slot_generation<ReuseSlots>(slotMetadata);
+
 		// Ensure that the element pointed-to by this slot is dead.
 		slotMetadata.set_dead();
 
@@ -579,7 +589,7 @@ class PagedEntityContainer
 
 	const_iterator begin() const
 	{
-		return const_iterator(const_cast<PagedEntityContainer<T>*>(this)->begin());
+		return const_iterator(const_cast<PagedEntityContainer*>(this)->begin());
 	}
 
 	iterator begin()
@@ -656,7 +666,7 @@ class PagedEntityContainer
 
 	const_iterator find(const T& x) const
 	{
-		return const_iterator(const_cast<PagedEntityContainer<T>*>(this)->find(const_cast<T&>(x)));
+		return const_iterator(const_cast<PagedEntityContainer*>(this)->find(const_cast<T&>(x)));
 	}
 
 	void erase(const_iterator it)
@@ -682,7 +692,7 @@ class PagedEntityContainer
 		}
 		// Shrink the storage to just a single page.
 		_pages.resize(1);
-		_capacity = MAX_ELEMENTS_PER_PAGE;
+		_capacity = MaxElementsPerPage;
 		// No valid items in the container now.
 		_maxIndex = INVALID_SLOT_IDX;
 		_pages.front().reset_metadata();
@@ -742,12 +752,12 @@ class PagedEntityContainer
 
 	static PageIndex global_to_page_index(SlotIndexType idx)
 	{
-		return {idx / MAX_ELEMENTS_PER_PAGE, idx % MAX_ELEMENTS_PER_PAGE};
+		return { idx / MaxElementsPerPage, idx % MaxElementsPerPage };
 	}
 
 	static SlotIndexType page_index_to_global(const PageIndex& idx)
 	{
-		return idx.first * MAX_ELEMENTS_PER_PAGE + idx.second;
+		return idx.first * MaxElementsPerPage + idx.second;
 	}
 
 	// No need to call destructors manually for trivially destructible types.
@@ -786,16 +796,32 @@ class PagedEntityContainer
 		return (reinterpret_cast<uintptr_t>(addr) % alignof(T)) == 0;
 	}
 
+	template <bool ReuseSlotsAux = ReuseSlots, std::enable_if_t<ReuseSlotsAux, bool> = true>
+	void advance_slot_generation(SlotMetadata& meta)
+	{
+		// Advance slot generation number, when `ReuseSlots=true`.
+		meta.advance_generation();
+	}
+
+	// Specialization for the case when `ReuseSlots=false`.
+	template <bool ReuseSlotsAux = ReuseSlots, std::enable_if_t<!ReuseSlotsAux, bool> = true>
+	void advance_slot_generation(SlotMetadata& meta)
+	{
+		// Invalidate slot right away so that it cannot be reused anymore.
+		meta.invalidate();
+	}
+
 	std::vector<Page> _pages;
 	SlotIndexType _maxIndex = INVALID_SLOT_IDX;
 	size_t _size = 0;
 	size_t _capacity = 0;
 	size_t _expiredSlotsCount = 0;
 };
 
-template <typename T>
-constexpr typename PagedEntityContainer<T>::SlotIndexType PagedEntityContainer<T>::INVALID_SLOT_IDX;
+template <typename T, size_t MaxElementsPerPage, bool ReuseSlots>
+constexpr typename PagedEntityContainer<T, MaxElementsPerPage, ReuseSlots>::SlotIndexType
+	PagedEntityContainer<T, MaxElementsPerPage, ReuseSlots>::INVALID_SLOT_IDX;
 
-template <typename T>
-constexpr size_t PagedEntityContainer<T>::INVALID_PAGE_IDX;
+template <typename T, size_t MaxElementsPerPage, bool ReuseSlots>
+constexpr size_t PagedEntityContainer<T, MaxElementsPerPage, ReuseSlots>::INVALID_PAGE_IDX;
 

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 */

src/droid.cpp

@@ -252,7 +252,7 @@
 			{
 				// FIXME: change when adding submarines to the game
 //				modifiedModelMatrix *= glm::translate(glm::vec3(0.f, -world_coord(1) / 2.3f, 0.f));
 				position.y += (world_coord(1) / 2.3f);
 			}
 
 			EFFECT_TYPE type = DROID_ANIMEVENT_DYING_NORMAL;
@@ -444,14 +444,17 @@
 		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 @@
 
 	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 @@
 	}
 
 	// 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<void *>(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<void *>(&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 @@
 {
 	return &asConstructStats[asBits[COMP_CONSTRUCT]];
 }
+
+DroidContainer& GlobalDroidContainer()
+{
+	static DroidContainer instance;
+	return instance;
+}

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<DROID, 256, false>;
+DroidContainer& GlobalDroidContainer();
+
 #endif // __INCLUDED_SRC_DROID_H__