From 4bf95703a0a43089395094f167e3cb5c007e2da2 Mon Sep 17 00:00:00 2001
From: lhofhansl <larsh@apache.org>
Date: Wed, 10 Jan 2024 09:17:26 -0800
Subject: [PATCH] Allow access into MapSector::m_blocks (#14232)

* New API to allow access into MapSector::m_blocks
* Use this API on ClientMap::touchMapBlocks(), ClientMap::updateDrawList(), and ClientMap::updateDrawListShadow() to speed them up
---
 src/client/clientmap.cpp | 24 +++++++++---------------
 src/mapsector.h          | 10 +++++++++-
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/client/clientmap.cpp b/src/client/clientmap.cpp
index 7224c6490..419c43d40 100644
--- a/src/client/clientmap.cpp
+++ b/src/client/clientmap.cpp
@@ -329,7 +329,7 @@ void ClientMap::updateDrawList()
 		MapBlockVect sectorblocks;
 
 		for (auto &sector_it : m_sectors) {
-			MapSector *sector = sector_it.second;
+			const MapSector *sector = sector_it.second;
 			v2s16 sp = sector->getPos();
 
 			blocks_loaded += sector->size();
@@ -339,11 +339,9 @@ void ClientMap::updateDrawList()
 					continue;
 			}
 
-			sectorblocks.clear();
-			sector->getBlocks(sectorblocks);
-
 			// Loop through blocks in sector
-			for (MapBlock *block : sectorblocks) {
+			for (const auto &entry : sector->getBlocks()) {
+				MapBlock *block = entry.second.get();
 				MapBlockMesh *mesh = block->mesh;
 
 				// Calculate the coordinates for range and frustum culling
@@ -649,7 +647,7 @@ void ClientMap::touchMapBlocks()
 	u32 blocks_in_range_with_mesh = 0;
 
 	for (const auto &sector_it : m_sectors) {
-		MapSector *sector = sector_it.second;
+		const MapSector *sector = sector_it.second;
 		v2s16 sp = sector->getPos();
 
 		blocks_loaded += sector->size();
@@ -659,14 +657,12 @@ void ClientMap::touchMapBlocks()
 				continue;
 		}
 
-		MapBlockVect sectorblocks;
-		sector->getBlocks(sectorblocks);
-
 		/*
 			Loop through blocks in sector
 		*/
 
-		for (MapBlock *block : sectorblocks) {
+		for (const auto &entry : sector->getBlocks()) {
+			MapBlock *block = entry.second.get();
 			MapBlockMesh *mesh = block->mesh;
 
 			// Calculate the coordinates for range and frustum culling
@@ -1266,18 +1262,16 @@ void ClientMap::updateDrawListShadow(v3f shadow_light_pos, v3f shadow_light_dir,
 	u32 blocks_in_range_with_mesh = 0;
 
 	for (auto &sector_it : m_sectors) {
-		MapSector *sector = sector_it.second;
+		const MapSector *sector = sector_it.second;
 		if (!sector)
 			continue;
 		blocks_loaded += sector->size();
 
-		MapBlockVect sectorblocks;
-		sector->getBlocks(sectorblocks);
-
 		/*
 			Loop through blocks in sector
 		*/
-		for (MapBlock *block : sectorblocks) {
+		for (const auto &entry : sector->getBlocks()) {
+			MapBlock *block = entry.second.get();
 			MapBlockMesh *mesh = block->mesh;
 			if (!mesh) {
 				// Ignore if mesh doesn't exist
diff --git a/src/mapsector.h b/src/mapsector.h
index ee8a2c16f..13aa6907f 100644
--- a/src/mapsector.h
+++ b/src/mapsector.h
@@ -45,7 +45,7 @@ public:
 
 	void deleteBlocks();
 
-	v2s16 getPos()
+	v2s16 getPos() const
 	{
 		return m_pos;
 	}
@@ -62,8 +62,16 @@ public:
 	// Returns an owning ptr to block.
 	std::unique_ptr<MapBlock> detachBlock(MapBlock *block);
 
+	// This makes a copy of the internal collection.
+	// Prefer getBlocks() if possible.
 	void getBlocks(MapBlockVect &dest);
 
+	// Get access to the internal collection
+	// This is explicitly only allowed on a const object since modifying anything while iterating is unsafe.
+	// The caller needs to make sure that this does not happen.
+	const auto &getBlocks() const { return m_blocks; }
+	const auto &getBlocks() = delete;
+
 	bool empty() const { return m_blocks.empty(); }
 
 	int size() const { return m_blocks.size(); }