From 43aad3711b21cde371089b1e6136bf5472724019 Mon Sep 17 00:00:00 2001 From: lhofhansl Date: Sun, 29 Jun 2025 13:36:47 -0700 Subject: [PATCH] MapBlock::getData be gone (#16292) * Remove Mapblock::getData and all its uses * Do not leak ystride, zstride, and nodecount --- src/dummymap.h | 8 +++++--- src/mapblock.cpp | 5 ++--- src/mapblock.h | 13 +++++++------ src/unittest/test_mapblock.cpp | 28 +++++++++++++++++++--------- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/dummymap.h b/src/dummymap.h index 408f91341..942612604 100644 --- a/src/dummymap.h +++ b/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 (size_t i = 0; i < MapBlock::nodecount; i++) - data[i] = n; + for (s16 zn=0; zn < MAP_BLOCKSIZE; zn++) + for (s16 yn=0; yn < MAP_BLOCKSIZE; yn++) + for (s16 xn=0; xn < MAP_BLOCKSIZE; xn++) { + block->setNodeNoCheck(xn, yn, zn, n); + } block->expireIsAirCache(); } } diff --git a/src/mapblock.cpp b/src/mapblock.cpp index 348c02a1e..68cc0aeec 100644 --- a/src/mapblock.cpp +++ b/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(); diff --git a/src/mapblock.h b/src/mapblock.h index 9a13b686f..9778b98b2 100644 --- a/src/mapblock.h +++ b/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 diff --git a/src/unittest/test_mapblock.cpp b/src/unittest/test_mapblock.cpp index e4071efd5..1d26b40a8 100644 --- a/src/unittest/test_mapblock.cpp +++ b/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] = - MapNode(rval % max, (rval >> 16) & 0xff, (rval >> 24) & 0xff); + block.setNodeNoCheck(x, y, z, + 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) - block.getData()[i] = MapNode(CONTENT_AIR); + for (s16 z=0; z < MAP_BLOCKSIZE; z++) + 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) - UASSERT(block.getData()[i].getContent() != CONTENT_IGNORE); + for (s16 z=0; z < MAP_BLOCKSIZE; z++) + 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});