Merge 3d8dad2cc6 into 7bb4940ebd

Removes manual memory management with new/delete.

src/ai/ai_instance.cpp

@@ -83,7 +83,7 @@ void AIInstance::Died()
 
 void AIInstance::LoadDummyScript()
 {
-	ScriptAllocatorScope alloc_scope(this->engine);
+	ScriptAllocatorScope alloc_scope(this->engine.get());
 	Script_CreateDummy(this->engine->GetVM(), STR_ERROR_AI_NO_AI_FOUND, "AI");
 }
 

src/core/strong_typedef_type.hpp

@@ -158,4 +158,31 @@ namespace StrongType {
 	};
 }
 
+/**
+ * Implementation of std::hash for StrongType::Typedef.
+ *
+ * This specialization of std::hash allows hashing of StrongType::Typedef instances
+ * by leveraging the hash of the base type.
+ *
+ * Example Usage:
+ *   using MyType = StrongType::Typedef<int, struct MyTypeTag>;
+ *   std::unordered_map<MyType, std::string> my_map;
+ *
+ * @tparam TBaseType The underlying type of the StrongType::Typedef.
+ * @tparam TProperties Additional properties for the StrongType::Typedef.
+ */
+template <typename TBaseType, typename... TProperties>
+struct std::hash<StrongType::Typedef<TBaseType, TProperties...>> {
+	/**
+	 * Computes the hash value for a StrongType::Typedef instance.
+	 *
+	 * @param t The StrongType::Typedef instance to hash.
+	 * @return The hash value of the base type of t.
+	 */
+	std::size_t operator()(const StrongType::Typedef<TBaseType, TProperties...> &t) const noexcept
+	{
+		return std::hash<TBaseType>()(t.base());
+	}
+};
+
 #endif /* STRONG_TYPEDEF_TYPE_HPP */

src/landscape.cpp

@@ -27,7 +27,6 @@
 #include "effectvehicle_func.h"
 #include "landscape_type.h"
 #include "animated_tile_func.h"
-#include "core/flatset_type.hpp"
 #include "core/random_func.hpp"
 #include "object_base.h"
 #include "company_func.h"
@@ -43,6 +42,8 @@
 #include "table/strings.h"
 #include "table/sprites.h"
 
+#include <unordered_set>
+
 #include "safeguards.h"
 
 extern const TileTypeProcs
@@ -1298,28 +1299,26 @@ public:
  */
 static std::tuple<bool, bool> FlowRiver(TileIndex spring, TileIndex begin, uint min_river_length)
 {
-	uint height_begin = TileHeight(begin);
-
 	if (IsWaterTile(begin)) {
 		return { DistanceManhattan(spring, begin) > min_river_length, GetTileZ(begin) == 0 };
 	}
 
-	FlatSet<TileIndex> marks;
+	int height_begin = TileHeight(begin);
+
+	std::unordered_set<TileIndex> marks;
 	marks.insert(begin);
 
-	/* Breadth first search for the closest tile we can flow down to. */
-	std::list<TileIndex> queue;
+	std::vector<TileIndex> queue;
 	queue.push_back(begin);
 
+	/* Breadth first search for the closest tile we can flow down to. */
 	bool found = false;
-	uint count = 0; // Number of tiles considered; to be used for lake location guessing.
 	TileIndex end;
-	do {
-		end = queue.front();
-		queue.pop_front();
+	for (size_t i = 0; i != queue.size(); i++) {
+		end = queue[i];
 
-		uint height_end = TileHeight(end);
-		if (IsTileFlat(end) && (height_end < height_begin || (height_end == height_begin && IsWaterTile(end)))) {
+		int height_end;
+		if (IsTileFlat(end, &height_end) && (height_end < height_begin || (height_end == height_begin && IsWaterTile(end)))) {
 			found = true;
 			break;
 		}
@@ -1328,31 +1327,29 @@ static std::tuple<bool, bool> FlowRiver(TileIndex spring, TileIndex begin, uint
 			TileIndex t = end + TileOffsByDiagDir(d);
 			if (IsValidTile(t) && !marks.contains(t) && FlowsDown(end, t)) {
 				marks.insert(t);
-				count++;
 				queue.push_back(t);
 			}
 		}
-	} while (!queue.empty());
+	}
 
 	bool main_river = false;
 	if (found) {
 		/* Flow further down hill. */
 		std::tie(found, main_river) = FlowRiver(spring, end, min_river_length);
-	} else if (count > 32) {
+	} else if (queue.size() > 32) {
 		/* Maybe we can make a lake. Find the Nth of the considered tiles. */
-		auto cit = marks.cbegin();
-		std::advance(cit, RandomRange(count - 1));
-		TileIndex lake_centre = *cit;
+		TileIndex lake_centre = queue[RandomRange(static_cast<uint32_t>(queue.size()))];
+		int height_lake;
 
 		if (IsValidTile(lake_centre) &&
-				/* A river, or lake, can only be built on flat slopes. */
-				IsTileFlat(lake_centre) &&
-				/* We want the lake to be built at the height of the river. */
-				TileHeight(begin) == TileHeight(lake_centre) &&
 				/* We don't want the lake at the entry of the valley. */
 				lake_centre != begin &&
 				/* We don't want lakes in the desert. */
 				(_settings_game.game_creation.landscape != LandscapeType::Tropic || GetTropicZone(lake_centre) != TROPICZONE_DESERT) &&
+				/* A river, or lake, can only be built on flat slopes. */
+				IsTileFlat(lake_centre, &height_lake) &&
+				/* We want the lake to be built at the height of the river. */
+				height_lake == height_begin &&
 				/* We only want a lake if the river is long enough. */
 				DistanceManhattan(spring, lake_centre) > min_river_length) {
 			end = lake_centre;
@@ -1362,7 +1359,7 @@ static std::tuple<bool, bool> FlowRiver(TileIndex spring, TileIndex begin, uint
 			/* Run the loop twice, so artefacts from going circular in one direction get (mostly) hidden. */
 			for (uint loops = 0; loops < 2; ++loops) {
 				for (auto tile : SpiralTileSequence(lake_centre, diameter)) {
-					MakeLake(tile, height_begin);
+					MakeLake(tile, height_lake);
 				}
 			}
 
@@ -1370,7 +1367,6 @@ static std::tuple<bool, bool> FlowRiver(TileIndex spring, TileIndex begin, uint
 		}
 	}
 
-	marks.clear();
 	if (found) RiverBuilder::Exec(begin, end, spring, main_river);
 	return { found, main_river };
 }

src/script/api/script_object.cpp

@@ -53,7 +53,7 @@ static ScriptStorage &GetStorage()
 
 /* static */ ScriptInstance *ScriptObject::ActiveInstance::active = nullptr;
 
-ScriptObject::ActiveInstance::ActiveInstance(ScriptInstance &instance) : alc_scope(instance.engine)
+ScriptObject::ActiveInstance::ActiveInstance(ScriptInstance &instance) : alc_scope(instance.engine.get())
 {
 	this->last_active = ScriptObject::ActiveInstance::active;
 	ScriptObject::ActiveInstance::active = &instance;
@@ -230,8 +230,8 @@ ScriptObject::DisableDoCommandScope::DisableDoCommandScope()
 
 /* static */ bool ScriptObject::CanSuspend()
 {
-	Squirrel *squirrel = ScriptObject::GetActiveInstance().engine;
-	return GetStorage().allow_do_command && squirrel->CanSuspend();
+	Squirrel &squirrel = *ScriptObject::GetActiveInstance().engine;
+	return GetStorage().allow_do_command && squirrel.CanSuspend();
 }
 
 /* static */ ScriptEventQueue &ScriptObject::GetEventQueue()

src/script/script_instance.cpp

@@ -50,8 +50,8 @@ static void PrintFunc(bool error_msg, std::string_view message)
 
 ScriptInstance::ScriptInstance(std::string_view api_name)
 {
-	this->storage = new ScriptStorage();
-	this->engine  = new Squirrel(api_name);
+	this->storage = std::make_unique<ScriptStorage>();
+	this->engine = std::make_unique<Squirrel>(api_name);
 	this->engine->SetPrintFunction(&PrintFunc);
 }
 
@@ -59,10 +59,10 @@ void ScriptInstance::Initialize(const std::string &main_script, const std::strin
 {
 	ScriptObject::ActiveInstance active(*this);
 
-	this->controller = new ScriptController(company);
+	this->controller = std::make_unique<ScriptController>(company);
 
 	/* Register the API functions and classes */
-	this->engine->SetGlobalPointer(this->engine);
+	this->engine->SetGlobalPointer(this->engine.get());
 	this->RegisterAPI();
 	if (this->IsDead()) {
 		/* Failed to register API; a message has already been logged. */
@@ -81,12 +81,11 @@ void ScriptInstance::Initialize(const std::string &main_script, const std::strin
 		}
 
 		/* Create the main-class */
-		this->instance = new SQObject();
-		if (!this->engine->CreateClassInstance(instance_name, this->controller, this->instance)) {
+		this->instance = std::make_unique<SQObject>();
+		if (!this->engine->CreateClassInstance(instance_name, this->controller.get(), this->instance.get())) {
 			/* If CreateClassInstance has returned false instance has not been
 			 * registered with squirrel, so avoid trying to Release it by clearing it now */
-			delete this->instance;
-			this->instance = nullptr;
+			this->instance.reset();
 			this->Died();
 			return;
 		}
@@ -158,11 +157,10 @@ ScriptInstance::~ScriptInstance()
 	ScriptObject::ActiveInstance active(*this);
 	this->in_shutdown = true;
 
-	if (instance != nullptr) this->engine->ReleaseObject(this->instance);
-	if (engine != nullptr) delete this->engine;
-	delete this->storage;
-	delete this->controller;
-	delete this->instance;
+	if (instance != nullptr) this->engine->ReleaseObject(this->instance.get());
+
+	/* Engine must be reset explicitly in scope of the active instance. */
+	this->engine.reset();
 }
 
 void ScriptInstance::Continue()
@@ -179,11 +177,9 @@ void ScriptInstance::Died()
 
 	this->last_allocated_memory = this->GetAllocatedMemory(); // Update cache
 
-	if (this->instance != nullptr) this->engine->ReleaseObject(this->instance);
-	delete this->instance;
-	delete this->engine;
-	this->instance = nullptr;
-	this->engine = nullptr;
+	if (this->instance != nullptr) this->engine->ReleaseObject(this->instance.get());
+	this->engine.reset();
+	this->instance.reset();
 }
 
 void ScriptInstance::GameLoop()

src/script/script_instance.hpp

@@ -256,7 +256,7 @@ public:
 	void ReleaseSQObject(HSQOBJECT *obj);
 
 protected:
-	class Squirrel *engine = nullptr; ///< A wrapper around the squirrel vm.
+	std::unique_ptr<class Squirrel> engine; ///< A wrapper around the squirrel vm.
 	std::string api_version{}; ///< Current API used by this script.
 
 	/**
@@ -288,9 +288,9 @@ protected:
 	virtual void LoadDummyScript() = 0;
 
 private:
-	class ScriptController *controller = nullptr; ///< The script main class.
-	class ScriptStorage *storage = nullptr; ///< Some global information for each running script.
-	SQObject *instance = nullptr; ///< Squirrel-pointer to the script main class.
+	std::unique_ptr<class ScriptStorage> storage; ///< Some global information for each running script.
+	std::unique_ptr<class ScriptController> controller; ///< The script main class.
+	std::unique_ptr<SQObject> instance; ///< Squirrel-pointer to the script main class.
 
 	bool is_started = false; ///< Is the scripts constructor executed?
 	bool is_dead = false; ///< True if the script has been stopped.