From a24318eedc44af2f5967c4727b7ebf578b8e233a Mon Sep 17 00:00:00 2001 From: ModMaker101 <119018978+ModMaker101@users.noreply.github.com> Date: Wed, 25 Mar 2026 00:25:18 -0400 Subject: [PATCH] Memory leak fix: Make chunks unload properly (#1406) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix chunk unload and cleanup logic, fixes #1347 * Applying formatting to code I edited 😝 --- Minecraft.Client/MultiPlayerChunkCache.cpp | 25 ++++++---- Minecraft.Client/PlayerChunkMap.cpp | 25 ++++++++++ Minecraft.Client/PlayerList.cpp | 11 ++++- Minecraft.Client/ServerChunkCache.cpp | 53 +++++++--------------- 4 files changed, 68 insertions(+), 46 deletions(-) diff --git a/Minecraft.Client/MultiPlayerChunkCache.cpp b/Minecraft.Client/MultiPlayerChunkCache.cpp index 62361ce3..03c47fcc 100644 --- a/Minecraft.Client/MultiPlayerChunkCache.cpp +++ b/Minecraft.Client/MultiPlayerChunkCache.cpp @@ -139,19 +139,26 @@ bool MultiPlayerChunkCache::reallyHasChunk(int x, int z) return hasData[idx]; } -void MultiPlayerChunkCache::drop(int x, int z) +void MultiPlayerChunkCache::drop(const int x, const int z) { - // 4J Stu - We do want to drop any entities in the chunks, especially for the case when a player is dead as they will - // not get the RemoveEntity packet if an entity is removed. - LevelChunk *chunk = getChunk(x, z); - if (!chunk->isEmpty()) + const int ix = x + XZOFFSET; + const int iz = z + XZOFFSET; + if ((ix < 0) || (ix >= XZSIZE)) return; + if ((iz < 0) || (iz >= XZSIZE)) return; + const int idx = ix * XZSIZE + iz; + LevelChunk* chunk = cache[idx]; + + if (chunk != nullptr && !chunk->isEmpty()) { - // Added parameter here specifies that we don't want to delete tile entities, as they won't get recreated unless they've got update packets - // The tile entities are in general only created on the client by virtue of the chunk rebuild + // Unload chunk but keep tile entities chunk->unload(false); - // 4J - We just want to clear out the entities in the chunk, but everything else should be valid - chunk->loaded = true; + const auto it = std::find(loadedChunkList.begin(), loadedChunkList.end(), chunk); + if (it != loadedChunkList.end()) loadedChunkList.erase(it); + + cache[idx] = nullptr; + hasData[idx] = false; + chunk->loaded = false; } } diff --git a/Minecraft.Client/PlayerChunkMap.cpp b/Minecraft.Client/PlayerChunkMap.cpp index bcc3f6ba..ddf2bae2 100644 --- a/Minecraft.Client/PlayerChunkMap.cpp +++ b/Minecraft.Client/PlayerChunkMap.cpp @@ -792,6 +792,14 @@ void PlayerChunkMap::setRadius(int newRadius) int xc = static_cast(player->x) >> 4; int zc = static_cast(player->z) >> 4; + for (auto it = addRequests.begin(); it != addRequests.end(); ) + { + if (it->player == player) + it = addRequests.erase(it); + else + ++it; + } + for (int x = xc - newRadius; x <= xc + newRadius; x++) for (int z = zc - newRadius; z <= zc + newRadius; z++) { @@ -801,9 +809,26 @@ void PlayerChunkMap::setRadius(int newRadius) getChunkAndAddPlayer(x, z, player); } } + + // Remove chunks that are outside the new radius + for (int x = xc - radius; x <= xc + radius; x++) + { + for (int z = zc - radius; z <= zc + radius; z++) + { + if (x < xc - newRadius || x > xc + newRadius || z < zc - newRadius || z > zc + newRadius) + { + getChunkAndRemovePlayer(x, z, player); + } + } + } } } + if (newRadius < radius) + { + level->cache->dropAll(); + } + assert(radius <= MAX_VIEW_DISTANCE); assert(radius >= MIN_VIEW_DISTANCE); this->radius = newRadius; diff --git a/Minecraft.Client/PlayerList.cpp b/Minecraft.Client/PlayerList.cpp index ba82ec6a..331539cb 100644 --- a/Minecraft.Client/PlayerList.cpp +++ b/Minecraft.Client/PlayerList.cpp @@ -1690,7 +1690,16 @@ bool PlayerList::isXuidBanned(PlayerUID xuid) } // AP added for Vita so the range can be increased once the level starts -void PlayerList::setViewDistance(int newViewDistance) +void PlayerList::setViewDistance(const int newViewDistance) { viewDistance = newViewDistance; + + for (size_t i = 0; i < server->levels.length; i++) + { + ServerLevel* level = server->levels[i]; + if (level != nullptr) + { + level->getChunkMap()->setRadius(newViewDistance); + } + } } diff --git a/Minecraft.Client/ServerChunkCache.cpp b/Minecraft.Client/ServerChunkCache.cpp index c7d70c7d..54312ffa 100644 --- a/Minecraft.Client/ServerChunkCache.cpp +++ b/Minecraft.Client/ServerChunkCache.cpp @@ -80,54 +80,31 @@ vector *ServerChunkCache::getLoadedChunkList() return &m_loadedChunkList; } -void ServerChunkCache::drop(int x, int z) +void ServerChunkCache::drop(const int x, const int z) { - // 4J - we're not dropping things anymore now that we have a fixed sized cache -#ifdef _LARGE_WORLDS + const int ix = x + XZOFFSET; + const int iz = z + XZOFFSET; + if ((ix < 0) || (ix >= XZSIZE)) return; + if ((iz < 0) || (iz >= XZSIZE)) return; + const int idx = ix * XZSIZE + iz; + LevelChunk* chunk = cache[idx]; - bool canDrop = false; -// if (level->dimension->mayRespawn()) -// { -// Pos *spawnPos = level->getSharedSpawnPos(); -// int xd = x * 16 + 8 - spawnPos->x; -// int zd = z * 16 + 8 - spawnPos->z; -// delete spawnPos; -// int r = 128; -// if (xd < -r || xd > r || zd < -r || zd > r) -// { -// canDrop = true; -//} -// } -// else + if (chunk != nullptr) { - canDrop = true; - } - if(canDrop) - { - int ix = x + XZOFFSET; - int iz = z + XZOFFSET; - // Check we're in range of the stored level - if( ( ix < 0 ) || ( ix >= XZSIZE ) ) return; - if( ( iz < 0 ) || ( iz >= XZSIZE ) ) return; - int idx = ix * XZSIZE + iz; - LevelChunk *chunk = cache[idx]; + const auto it = std::find(m_loadedChunkList.begin(), m_loadedChunkList.end(), chunk); + if (it != m_loadedChunkList.end()) m_loadedChunkList.erase(it); - if(chunk) - { - m_toDrop.push_back(chunk); - } + cache[idx] = nullptr; + chunk->loaded = false; } -#endif } void ServerChunkCache::dropAll() { -#ifdef _LARGE_WORLDS for (LevelChunk *chunk : m_loadedChunkList) { drop(chunk->x, chunk->z); -} -#endif + } } // 4J - this is the original (and virtual) interface to create @@ -957,6 +934,10 @@ bool ServerChunkCache::tick() m_unloadedCache[idx] = chunk; cache[idx] = nullptr; } + else + { + continue; + } } m_toDrop.pop_front(); }