From 62cea65da37b70becf63439bfa1e53aa3d70c498 Mon Sep 17 00:00:00 2001
From: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
Date: Tue, 12 Mar 2024 01:13:14 +0300
Subject: [PATCH] Switch `FEATURE` to use `PagedEntityContainer<FEATURE>` as
 backing storage

1. Split the feature storage into pages containing 128 features
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 <pavel.al.solodovnikov@gmail.com>
---
 src/feature.cpp | 16 +++++++++-------
 src/feature.h   |  7 +++++++
 src/objmem.cpp  | 33 ++++++++++++++++++++++++++++-----
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/src/feature.cpp b/src/feature.cpp
index 52f2382f4d7..1229336aa7d 100644
--- a/src/feature.cpp
+++ b/src/feature.cpp
@@ -183,14 +183,10 @@ FEATURE *buildFeature(FEATURE_STATS *psStats, UDWORD x, UDWORD y, bool FromSave)
 /* Create a feature on the map */
 FEATURE *buildFeature(FEATURE_STATS *psStats, UDWORD x, UDWORD y, bool FromSave, uint32_t id)
 {
-	//try and create the Feature
-	FEATURE *psFeature = new FEATURE(id, psStats);
+	//try and create the Feature, obtain stable address.
+	FEATURE& feature = GlobalFeatureContainer().emplace(id, psStats);
+	FEATURE* psFeature = &feature;
 
-	if (psFeature == nullptr)
-	{
-		debug(LOG_WARNING, "Feature couldn't be built.");
-		return nullptr;
-	}
 	//add the feature to the list - this enables it to be drawn whilst being built
 	addFeature(psFeature);
 
@@ -556,3 +552,9 @@ StructureBounds getStructureBounds(FEATURE_STATS const *stats, Vector2i pos)
 	const Vector2i map = map_coord(pos) - size / 2;
 	return StructureBounds(map, size);
 }
+
+FeatureContainer& GlobalFeatureContainer()
+{
+	static FeatureContainer instance;
+	return instance;
+}
diff --git a/src/feature.h b/src/feature.h
index b654bf54560..d8ffd03e536 100644
--- a/src/feature.h
+++ b/src/feature.h
@@ -26,6 +26,7 @@
 
 #include "objectdef.h"
 #include "lib/framework/wzconfig.h"
+#include "lib/framework/paged_entity_container.h"
 
 /* The statistics for the features */
 extern std::vector<FEATURE_STATS> asFeatureStats;
@@ -82,4 +83,10 @@ static inline FEATURE const *castFeature(SIMPLE_OBJECT const *psObject)
 	return isFeature(psObject) ? (FEATURE const *)psObject : (FEATURE const *)nullptr;
 }
 
+// Split the feature storage into pages containing 128 features, 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 FeatureContainer = PagedEntityContainer<FEATURE, 128, false>;
+FeatureContainer& GlobalFeatureContainer();
+
 #endif // __INCLUDED_SRC_FEATURE_H__
diff --git a/src/objmem.cpp b/src/objmem.cpp
index fc2870b7653..6d6974d5381 100644
--- a/src/objmem.cpp
+++ b/src/objmem.cpp
@@ -86,10 +86,9 @@ bool objmemInitialise()
 /* Release the object heaps */
 void objmemShutdown()
 {
-	auto& droidContainer = GlobalDroidContainer();
-	droidContainer.clear();
-	auto& structContainer = GlobalStructContainer();
-	structContainer.clear();
+	GlobalDroidContainer().clear();
+	GlobalStructContainer().clear();
+	GlobalFeatureContainer().clear();
 }
 
 // Check that psVictim is not referred to by any other object in the game. We can dump out some extra data in debug builds that help track down sources of dangling pointer errors.
@@ -244,6 +243,14 @@ bool objmemDestroy(BASE_OBJECT *psObj, bool checkRefs)
 		ASSERT(it != structContainer.end(), "Structure not found in the global container!");
 		structContainer.erase(it);
 	}
+	else if (psObj->type == OBJ_FEATURE)
+	{
+		// Features are managed by a separate feature container.
+		auto& featureContainer = GlobalFeatureContainer();
+		auto it = featureContainer.find(*static_cast<FEATURE*>(psObj));
+		ASSERT(it != featureContainer.end(), "Feature not found in the global container!");
+		featureContainer.erase(it);
+	}
 	else
 	{
 		delete psObj;
@@ -497,6 +504,22 @@ struct GlobalEntityContainerTraits<STRUCTURE>
 	}
 };
 
+template <>
+struct GlobalEntityContainerTraits<FEATURE>
+{
+	using StorageType = FeatureContainer;
+
+	static FeatureContainer& getContainer()
+	{
+		return GlobalFeatureContainer();
+	}
+
+	static const char* entityName()
+	{
+		return "Feature";
+	}
+};
+
 template <typename Entity, unsigned PlayerCount>
 static void freeAllEntitiesImpl(PerPlayerObjectLists<Entity, PlayerCount>& entityLists)
 {
@@ -692,7 +715,7 @@ void killFeature(FEATURE *psDel)
 /* Remove all features */
 void freeAllFeatures()
 {
-	releaseAllObjectsInList(apsFeatureLists);
+	freeAllEntitiesImpl<FEATURE, MAX_PLAYERS>(apsFeatureLists);
 }
 
 /**************************  FLAG_POSITION ********************************/