MapBlock::getData be gone (#16292)

* Remove Mapblock::getData and all its uses

* Do not leak ystride, zstride, and nodecount

src/dummymap.h

@@ -31,9 +31,11 @@ public:
 		for (s16 y = bpmin.Y; y <= bpmax.Y; y++) {
 			MapBlock *block = getBlockNoCreateNoEx({x, y, z});
 			if (block) {
-				auto *data = block->getData();
+				for (s16 zn=0; zn < MAP_BLOCKSIZE; zn++)
-				for (size_t i = 0; i < MapBlock::nodecount; i++)
+				for (s16 yn=0; yn < MAP_BLOCKSIZE; yn++)
-					data[i] = n;
+				for (s16 xn=0; xn < MAP_BLOCKSIZE; xn++) {
+					block->setNodeNoCheck(xn, yn, zn, n);
+				}
 				block->expireIsAirCache();
 			}
 		}

src/mapblock.cpp

@@ -12,7 +12,6 @@
 #include "gamedef.h"
 #include "irrlicht_changes/printing.h"
 #include "log.h"
-#include "nameidmapping.h"
 #include "content_mapnode.h"  // For legacy name-id mapping
 #include "content_nodemeta.h" // For legacy deserialization
 #include "serialization.h"
@@ -258,7 +257,7 @@ void MapBlock::expireIsAirCache()
 // Renumbers the content IDs (starting at 0 and incrementing)
 // Note that there's no technical reason why we *have to* renumber the IDs,
 // but we do it anyway as it also helps compressability.
-static void getBlockNodeIdMapping(NameIdMapping *nimap, MapNode *nodes,
+void MapBlock::getBlockNodeIdMapping(NameIdMapping *nimap, MapNode *nodes,
 	const NodeDefManager *nodedef)
 {
 	IdIdMapping &mapping = IdIdMapping::giveClearedThreadLocalInstance();
@@ -288,7 +287,7 @@ static void getBlockNodeIdMapping(NameIdMapping *nimap, MapNode *nodes,
 // Correct ids in the block to match nodedef based on names.
 // Unknown ones are added to nodedef.
 // Will not update itself to match id-name pairs in nodedef.
-static void correctBlockNodeIds(const NameIdMapping *nimap, MapNode *nodes,
+void MapBlock::correctBlockNodeIds(const NameIdMapping *nimap, MapNode *nodes,
 		IGameDef *gamedef)
 {
 	const NodeDefManager *nodedef = gamedef->ndef();

src/mapblock.h

@@ -21,6 +21,7 @@ class NodeMetadataList;
 class IGameDef;
 class MapBlockMesh;
 class VoxelManipulator;
+class NameIdMapping;
 
 #define BLOCK_TIMESTAMP_UNDEFINED 0xffffffff
 
@@ -80,11 +81,6 @@ public:
 		raiseModified(MOD_STATE_WRITE_NEEDED, MOD_REASON_REALLOCATE);
 	}
 
-	MapNode* getData()
-	{
-		return data;
-	}
-
 	////
 	//// Modification tracking methods
 	////
@@ -427,18 +423,23 @@ public:
 	// clearObject and return removed objects count
 	u32 clearObjects();
 
+private:
 	static const u32 ystride = MAP_BLOCKSIZE;
 	static const u32 zstride = MAP_BLOCKSIZE * MAP_BLOCKSIZE;
 
 	static const u32 nodecount = MAP_BLOCKSIZE * MAP_BLOCKSIZE * MAP_BLOCKSIZE;
 
-private:
 	/*
 		Private methods
 	*/
 
 	void deSerialize_pre22(std::istream &is, u8 version, bool disk);
 
+	static void getBlockNodeIdMapping(NameIdMapping *nimap, MapNode *nodes,
+		const NodeDefManager *nodedef);
+	static void correctBlockNodeIds(const NameIdMapping *nimap, MapNode *nodes,
+			IGameDef *gamedef);
+
 	/*
 	 * PLEASE NOTE: When adding something here be mindful of position and size
 	 * of member variables! This is also the reason for the weird public-private

src/unittest/test_mapblock.cpp

@@ -68,10 +68,12 @@ void TestMapBlock::testSaveLoad(IGameDef *gamedef, const u8 version)
 		MapBlock block({}, gamedef);
 		// Fill with data
 		PcgRandom r(seed);
-		for (size_t i = 0; i < MapBlock::nodecount; ++i) {
+		for (s16 z=0; z < MAP_BLOCKSIZE; z++)
+		for (s16 y=0; y < MAP_BLOCKSIZE; y++)
+		for (s16 x=0; x < MAP_BLOCKSIZE; x++) {
 			u32 rval = r.next();
-			block.getData()[i] =
+			block.setNodeNoCheck(x, y, z,
-				MapNode(rval % max, (rval >> 16) & 0xff, (rval >> 24) & 0xff);
+					MapNode(rval % max, (rval >> 16) & 0xff, (rval >> 24) & 0xff));
 		}
 
 		// Serialize
@@ -85,11 +87,13 @@ void TestMapBlock::testSaveLoad(IGameDef *gamedef, const u8 version)
 
 		// Check data
 		PcgRandom r(seed);
-		for (size_t i = 0; i < MapBlock::nodecount; ++i) {
+		for (s16 z=0; z < MAP_BLOCKSIZE; z++)
+		for (s16 y=0; y < MAP_BLOCKSIZE; y++)
+		for (s16 x=0; x < MAP_BLOCKSIZE; x++) {
 			u32 rval = r.next();
 			auto expect =
 				MapNode(rval % max, (rval >> 16) & 0xff, (rval >> 24) & 0xff);
-			UASSERT(block.getData()[i] == expect);
+			UASSERT(block.getNodeNoCheck(x, y, z) == expect);
 		}
 	}
 }
@@ -104,8 +108,11 @@ void TestMapBlock::testSave29(IGameDef *gamedef)
 	{
 		// Prepare test block
 		MapBlock block({}, gamedef);
-		for (size_t i = 0; i < MapBlock::nodecount; ++i)
+		for (s16 z=0; z < MAP_BLOCKSIZE; z++)
-			block.getData()[i] = MapNode(CONTENT_AIR);
+		for (s16 y=0; y < MAP_BLOCKSIZE; y++)
+		for (s16 x=0; x < MAP_BLOCKSIZE; x++) {
+			block.setNodeNoCheck(x, y, z, MapNode(CONTENT_AIR));
+		}
 		block.setNode({0, 0, 0}, MapNode(t_CONTENT_STONE));
 
 		block.serialize(ss, 29, true, -1);
@@ -294,8 +301,11 @@ void TestMapBlock::testLoad20(IGameDef *gamedef)
 	UASSERTEQ(auto, get_node(10, 6, 4), "air");
 	UASSERTEQ(auto, get_node(11, 6, 3), "default:furnace");
 
-	for (size_t i = 0; i < MapBlock::nodecount; ++i)
+	for (s16 z=0; z < MAP_BLOCKSIZE; z++)
-		UASSERT(block.getData()[i].getContent() != CONTENT_IGNORE);
+	for (s16 y=0; y < MAP_BLOCKSIZE; y++)
+	for (s16 x=0; x < MAP_BLOCKSIZE; x++) {
+		UASSERT(block.getNodeNoCheck(x, y, z).getContent() != CONTENT_IGNORE);
+	}
 
 	// metadata is also translated
 	auto *meta = block.m_node_metadata.get({11, 6, 3});